Skip to content
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

Issue #4047 Graceful Write #4100

Merged
merged 8 commits into from
Sep 25, 2019
Merged

Conversation

gregw
Copy link
Contributor

@gregw gregw commented Sep 18, 2019

This PR has been branched off #4094, which has started to address bigger graceful stop issues.
This PR address just the bug introduced in #2772:

  • Added test to reproduce issue
  • Fixed bug from Close on graceful #2749 #2772 where output was shutdown on DONE without checking for END.
  • Fixed aggregation logic to aggregate last write if aggregation already started.

Signed-off-by: Greg Wilkins gregw@webtide.com

Added test to reproduce issue
Fixed bug from #2772 where output was shutdown on DONE without checking for END.
Fixed aggregation logic to aggregate last write if aggregation already started.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw gregw added this to To do in Jetty 9.4.21 via automation Sep 19, 2019
@gregw gregw moved this from To do to In Review in Jetty 9.4.21 Sep 19, 2019
@gregw gregw requested a review from joakime September 22, 2019 22:41
Copy link
Contributor

@lachlan-roberts lachlan-roberts left a comment

Choose a reason for hiding this comment

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

Changes look good but I would like to see some javadoc on the Graceful interface as the comment there currently doesn't help to understand what it does.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
Reverted accidental commit and fixed javadoc error.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
Improved comment
Started stopper earlier

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw gregw requested a review from joakime September 24, 2019 02:32
@gregw
Copy link
Contributor Author

gregw commented Sep 24, 2019

@lachlan-roberts @janbartel can you both rerereview please?

Copy link
Contributor

@lachlan-roberts lachlan-roberts left a comment

Choose a reason for hiding this comment

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

LGTM

…47-graceful-ab-write

Signed-off-by: Greg Wilkins <gregw@webtide.com>
Improved comments and clarify conditions

Signed-off-by: Greg Wilkins <gregw@webtide.com>
Copy link
Contributor

@janbartel janbartel left a comment

Choose a reason for hiding this comment

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

LGTM

@gregw gregw merged commit 11b60db into jetty-9.4.x Sep 25, 2019
Jetty 9.4.21 automation moved this from In Review to Done Sep 25, 2019
@gregw gregw deleted the jetty-9.4.x-4047-graceful-ab-write branch September 25, 2019 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Jetty 9.4.21
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants