Skip to content

Comments

[Minor Doc Fix] Correct the default value of druid.server.http.gracefulShutdownTimeout#10661

Merged
jihoonson merged 3 commits intoapache:masterfrom
zhangyue19921010:fix-jettyServerModule-default-shutdown-timeout
Jan 8, 2021
Merged

[Minor Doc Fix] Correct the default value of druid.server.http.gracefulShutdownTimeout#10661
jihoonson merged 3 commits intoapache:masterfrom
zhangyue19921010:fix-jettyServerModule-default-shutdown-timeout

Conversation

@zhangyue19921010
Copy link
Contributor

@zhangyue19921010 zhangyue19921010 commented Dec 9, 2020

Description

According to the code

final long gracefulStop = config.getGracefulShutdownTimeout().toStandardDuration().getMillis();


The configuration druid.server.http.gracefulShutdownTimeout can only take effect when the user sets a value greater than zero. Otherwise this parameter will not be set, using the default value which is 30000 milliseconds(PT30S) coding in

https://github.com/eclipse/jetty.project/blob/jetty-9.4.34.v20201102/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java#L53


This PR has:

  • been self-reviewed.
Key changed/added classes in this PR
  • index.md

Copy link
Contributor

@a2l007 a2l007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

I guess if we indeed do want to support shutdown without waiting, we might need another config property to toggle setStopAtShutdown. But I doubt the need for such a config as druid.server.http.gracefulShutdownTimeout satisfies most of the usecases.

@zhangyue19921010
Copy link
Contributor Author

@a2l007 Thanks for your review!

@jihoonson
Copy link
Contributor

I guess if we indeed do want to support shutdown without waiting, we might need another config property to toggle setStopAtShutdown. But I doubt the need for such a config as druid.server.http.gracefulShutdownTimeout satisfies most of the usecases.

I agree. The current config seems enough.

@jihoonson jihoonson merged commit 2837a9b into apache:master Jan 8, 2021
JulianJaffePinterest pushed a commit to JulianJaffePinterest/druid that referenced this pull request Jan 22, 2021
…fulShutdownTimeout` (apache#10661)

* done

* done

* done

Co-authored-by: yuezhang <yuezhang@freewheel.tv>
@zhangyue19921010 zhangyue19921010 deleted the fix-jettyServerModule-default-shutdown-timeout branch February 9, 2021 08:42
@jihoonson jihoonson added this to the 0.21.0 milestone Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants