Skip to content

[Merged by Bors] - Wind down the SSE thread when the client disconnects #1514

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - Wind down the SSE thread when the client disconnects #1514

wants to merge 1 commit into from

Conversation

@adaszko
Copy link
Contributor

@adaszko adaszko commented Aug 12, 2020 •
edited
Loading

These started to appear when I ^C curl -N http://localhost:5052/beacon/fork/stream: Aug 12 13:00:01.539 ERRO Couldn't stream piece hyper::Error(ChannelClosed), service: http

Something must have changed in hyper since SSE has been implemented because I'm sure I haven't seen those errors before.

This PR properly detects a closed SSE stream and cleans up.

@adaszko adaszko changed the title Wind down the SSE thread when the client disconnected Wind down the SSE thread when the client disconnects Aug 12, 2020
@adaszko adaszko added bug Something isn't working ready-for-review The code is ready for review labels Aug 12, 2020
paulhauner
paulhauner approved these changes Aug 13, 2020
View reviewed changes
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

Nice!

bors r+

@paulhauner paulhauner added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Aug 13, 2020
bors bot pushed a commit that referenced this pull request Aug 13, 2020
@adaszko
Wind down the SSE thread when the client disconnects (#1514)
05a8399 
These started to appear when I `^C` `curl -N http://localhost:5052/beacon/fork/stream`: `Aug 12 13:00:01.539 ERRO Couldn't stream piece hyper::Error(ChannelClosed), service: http`

Something must have changed in hyper since SSE has been implemented because I'm sure I haven't seen those errors before.

This PR properly detects a closed SSE stream and cleans up.
@bors
Copy link

bors bot commented Aug 13, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title Wind down the SSE thread when the client disconnects [Merged by Bors] - Wind down the SSE thread when the client disconnects Aug 13, 2020
@bors bors bot closed this Aug 13, 2020
paulhauner added a commit that referenced this pull request Aug 15, 2020
@paulhauner
Squash in no-hash-replay
48d15f2 
Squashed commit of the following:

commit f2590e3
Author: Paul Hauner <paul@paulhauner.com>
Date:   Sat Aug 15 12:47:06 2020 +1000

    Introduced ForcedFixedLenIter

commit 52ab576
Author: Paul Hauner <paul@paulhauner.com>
Date:   Sat Aug 15 12:46:53 2020 +1000

    Fix bug in state-root-less replay

commit 765edf0
Author: Paul Hauner <paul@paulhauner.com>
Date:   Sat Aug 15 12:46:42 2020 +1000

    Update comment

commit 64393a4
Author: Paul Hauner <paul@paulhauner.com>
Date:   Sat Aug 15 12:02:36 2020 +1000

    Reject known gossip blocks quicker

commit 2ab752b
Author: Paul Hauner <paul@paulhauner.com>
Date:   Sat Aug 15 11:43:29 2020 +1000

    Fix mistake in previous commit

commit f93a681
Author: Paul Hauner <paul@paulhauner.com>
Date:   Sat Aug 15 11:34:40 2020 +1000

    Avoid cloning caches for atomic db ops

commit 8113ce6
Author: Paul Hauner <paul@paulhauner.com>
Date:   Sat Aug 15 11:12:46 2020 +1000

    Allow for skipping state roots in block replay

commit 6aeb896
Author: Paul Hauner <paul@paulhauner.com>
Date:   Fri Aug 14 22:24:27 2020 +0000

    Commit Cargo.lock changes, add build scripts (#1521)

    ## Issue Addressed

    NA

    ## Proposed Changes

    This PR commits the `Cargo.lock` file so it does not indicate a dirty git tree in the version tag. This code should be used for the `v0.2.3` release.

    Also, adds a `Makefile` command to produce tarballs for upload on release.

    ## Additional Info

    NA

commit f4a7311
Author: Paul Hauner <paul@paulhauner.com>
Date:   Fri Aug 14 08:32:31 2020 +0000

    Update to v0.2.3 (#1519)

    ## Issue Addressed

    NA

    ## Proposed Changes

    Bump versions to v0.2.3.

    ## Additional Info

    NA

commit 619ad10
Author: Paul Hauner <paul@paulhauner.com>
Date:   Fri Aug 14 06:36:38 2020 +0000

    Restrict fork choice getters to finalized blocks (#1475)

    ## Issue Addressed

    - Resolves #1451

    ## Proposed Changes

    - Restricts the `contains_block` and `contains_block` so they only indicate a block is present if it descends from the finalized root. This helps to ensure that fork choice never points to a block that has been pruned from the database.
    - Resolves #1451
    - Before importing a block, double-check that its parent is known and a descendant of the finalized root.
    - Split a big, monolithic block verification test into smaller tests.

    ## Additional Notes

    I suspect there would be a craftier way to do the `is_descendant_of_finalized` check, but we're a bit tight on time now and we can optimize later if it starts showing in benches.

    ## TODO

    - [x] Tests

commit b0a3731
Author: Paul Hauner <paul@paulhauner.com>
Date:   Fri Aug 14 04:38:45 2020 +0000

    Introduce a queue for attestations from the network (#1511)

    ## Issue Addressed

    N/A

    ## Proposed Changes

    Introduces the `GossipProcessor`, a multi-threaded (multi-tasked?), non-blocking processor for some messages from the network which require verification and import into the `BeaconChain`.

    Initial testing indicates that this massively improves system stability by (a) moving block tasks from the normal executor (b) spreading out attestation load.

    ## Additional Info

    TBC

commit e3d45ed
Author: Pawan Dhananjay <pawandhananjay@gmail.com>
Date:   Thu Aug 13 07:00:37 2020 +0000

    Log to file without json format (#1485)

    ## Issue Addressed

    N/A

    ## Proposed Changes

    Earlier, to log to a file, the only options were to redirect stdout/stderr to a file or use json logging.
    Redirecting to stdout/stderr works well but causes issues with mistakenly overwriting the file instead of appending which has resulted in loss of precious logs on multiple occasions for me.

    Json logging creates a timestamped backup of the file if it already exists, but the json format itself is hugely annoying.

    This PR modifies the `--logfile` option to log as it does in the terminal to a logfile.

commit 05a8399
Author: Adam Szkoda <adaszko@gmail.com>
Date:   Thu Aug 13 06:12:18 2020 +0000

    Wind down the SSE thread when the client disconnects (#1514)

    These started to appear when I `^C` `curl -N http://localhost:5052/beacon/fork/stream`: `Aug 12 13:00:01.539 ERRO Couldn't stream piece hyper::Error(ChannelClosed), service: http`

    Something must have changed in hyper since SSE has been implemented because I'm sure I haven't seen those errors before.

    This PR properly detects a closed SSE stream and cleans up.

commit e6f4552
Author: ladidan <46406043+ladidan@users.noreply.github.com>
Date:   Thu Aug 13 05:25:51 2020 +0000

    Update key-management.md (#1515)

    ## Issue Addressed

    consequent use of "wally"

    ## Proposed Changes

    Please list or describe the changes introduced by this PR.

    ## Additional Info

    Please provide any additional information. For example, future considerations
    or information useful for reviewers.

commit 8a1a405
Author: Adam Szkoda <adaszko@gmail.com>
Date:   Wed Aug 12 07:00:00 2020 +0000

    Fix a bug in fork pruning (#1507)

    Extracted from #1380 because merging #1380 proves to be contentious.

    Co-authored-by: Michael Sproul <michael@sigmaprime.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@paulhauner paulhauner paulhauner approved these changes

Assignees

No one assigned

Labels

bug Something isn't working ready-for-merge This PR is ready to merge.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@adaszko @paulhauner