-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build: add "--with-service-timeout" in configure.ac #10242
Conversation
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 amd64 part 4: Failed (click for details)Topotests Ubuntu 18.04 amd64 part 4: No useful log foundSuccessful on other platforms/tests
|
Commit says 2 minutes, PR says 4 minutes, but the real change is 5 minutes 😄 Which is true? |
@ton31337 sorry for my mistake. |
I would recommend that we make this a configurable item instead of just changing it to an arbitrary time that works for slow systems. Why should we wait 5 minutes on a fast system if it's run into an issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see some sort of configure.ac change that allows us to set this at compile time to allow those operators to do the right thing for them instead of arbitrarily changing this TimeOutSec again.
822d24c
to
c27d559
Compare
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests debian 10 amd64 part 8: Failed (click for details)Topotests debian 10 amd64 part 8: No useful log foundTopotests Ubuntu 18.04 amd64 part 8: Failed (click for details)Topotests Ubuntu 18.04 amd64 part 8: No useful log foundTopotests debian 10 amd64 part 5: Failed (click for details)Topotests debian 10 amd64 part 5: No useful log foundSuccessful on other platforms/tests
|
@donaldsharp |
Agree with @donaldsharp. |
c27d559
to
fc8fc0d
Compare
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDTest incomplete. See below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: IncompleteTopotests Ubuntu 18.04 i386 part 3: Incomplete(check logs for details)IPv6 protocols on Ubuntu 18.04: Incomplete(check logs for details)IPv4 protocols on Ubuntu 18.04: Incomplete(check logs for details)Successful on other platforms/tests
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add this new configure option to doc/user/installation.rst. I'll get this in then
fc8fc0d
to
e472a96
Compare
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-2515/ This is a comment from an automated CI system. |
configure.ac
Outdated
@@ -932,6 +934,20 @@ AC_DEFINE_UNQUOTED([MULTIPATH_NUM], [$MPATH_NUM], [Maximum number of paths for a | |||
|
|||
AC_DEFINE_UNQUOTED([VTYSH_PAGER], ["$VTYSH_PAGER"], [What pager to use]) | |||
|
|||
|
|||
TIMEOUT_SEC=2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be minutes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK!I will use TIMEOUT_MIN.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
On lower CPU with lots of static routes, it will cost more than 2 minutes. 2 minutes is the default timeout value, we can adjust it by configure: ./configure --with-service-timeout=<digit> Signed-off-by: anlan_cs <anlan_cs@tom.com>
e472a96
to
fc53921
Compare
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-2541/ This is a comment from an automated CI system. |
build: add "--with-service-timeout" in configure.ac
On lower CPU with lots of static routes, it will cost more than 2
minutes.
2 minutes is the default timeout value, we can adjust it by configure:
./configure --with-service-timeout=
Signed-off-by: anlancs anlan_cs@tom.com