Implement event-based contexts for view changes and timeouts #74

johningve · 2022-08-19T11:28:59Z

This PR implements two functions, ViewContext and TimeoutContext. They return contexts that can be used for operations that should be canceled upon reaching a specific view, or when a timeout occurs.

Implementing these contexts require changes to the event loop. Specifically, we have to reintroduce asynchronous handlers, as the contexts must be able to react to new events to cancel tasks that may be running on the event loop itself. I chose to refactor the event loop handlers by adding some handler options. Currently, the only options are RunAsync and WithPriority. RunAsync makes the handler execute as part of AddEvent. WithPriority effectively turns the handler into an "observer". The idea is that handlers that don't require much time to run can use the WithPriority option to be executed before the other handlers. I am deprecating the Observer functionality in favor of the priority options.

If we are going to use a view context for voting, then we need to be in the correct view before we start the voting process. Otherwise, the vote could be canceled by the following view change.

This decouples contexts from the synchronizer and makes them listen for events on the event loop.

This commit aims to make the eventloop more flexible by allowing both async and sync handlers again. Handlers can be made async by passing a RunAsync handler option. Furthermore, observers have been replaced by a WithPriority handler option.

Smart contexts that listen for timeouts

Now options are set by calling a WithOptions method on which RegisterHandler can be called afterwards.

The options struct is used for configuration that is shared between modules. With this commit, we no longer need to add new options as a field in the options struct, but allow packages to register new options at will. The options are stored in an array and indexed using the option ID, so the performance impact should be minimal.

copylocks: <field> passes lock by value: github.com/relab/hotstuff/modules.Options contains sync.Mutex (govet)

codecov-commenter · 2023-05-03T00:01:20Z

Codecov Report

Merging #74 (5aa1494) into master (be433a2) will decrease coverage by 0.52%.
The diff coverage is 94.23%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master      #74      +/-   ##
==========================================
- Coverage   72.42%   71.91%   -0.52%     
==========================================
  Files          61       63       +2     
  Lines        6168     6305     +137     
==========================================
+ Hits         4467     4534      +67     
- Misses       1401     1458      +57     
- Partials      300      313      +13

Flag	Coverage Δ
unittests	`71.91% <94.23%> (-0.52%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
modules/modules.go	`100.00% <ø> (ø)`
backend/config.go	`74.87% <77.27%> (+0.61%)`	⬆️
modules/options.go	`88.63% <85.18%> (-3.04%)`	⬇️
util/gpool/gpool.go	`86.66% <86.66%> (ø)`
eventloop/eventloop.go	`87.96% <97.14%> (-5.38%)`	⬇️
blockchain/blockchain.go	`97.08% <100.00%> (ø)`
consensus/consensus.go	`74.33% <100.00%> (-2.52%)`	⬇️
handel/handel.go	`92.39% <100.00%> (-0.95%)`	⬇️
handel/session.go	`82.61% <100.00%> (-4.99%)`	⬇️
synchronizer/context.go	`100.00% <100.00%> (ø)`
... and 2 more

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

meling

Mostly nice, but please check my comments. We may need to discuss some of them.

meling · 2023-05-03T01:22:35Z

eventloop/eventloop.go

+}
+
+// UnregisterObserver unregister a handler.
+// Deprecated: use UnregisterHandler instead.


I think it is okay to just delete the method, since it is not used in the codebase.

eventloop/eventloop.go

modules/options.go

meling · 2023-05-03T23:51:26Z

synchronizer/context_test.go

+	eventloop.AddEvent(synchronizer.ViewChangeEvent{View: 0})
+
+	if ctx.Err() != nil {
+		t.Error("Context cancelled")


Shouldn't this check if the error was actually a context cancelled error?

synchronizer/synchronizer.go

eventloop/eventloop.go

If we are going to use a view context for voting, then we need to be in the correct view before we start the voting process. Otherwise, the vote could be canceled by the following view change.

This decouples contexts from the synchronizer and makes them listen for events on the event loop.

This commit aims to make the eventloop more flexible by allowing both async and sync handlers again. Handlers can be made async by passing a RunAsync handler option. Furthermore, observers have been replaced by a WithPriority handler option.

Smart contexts that listen for timeouts

Now options are set by calling a WithOptions method on which RegisterHandler can be called afterwards.

The options struct is used for configuration that is shared between modules. With this commit, we no longer need to add new options as a field in the options struct, but allow packages to register new options at will. The options are stored in an array and indexed using the option ID, so the performance impact should be minimal.

copylocks: <field> passes lock by value: github.com/relab/hotstuff/modules.Options contains sync.Mutex (govet)

This commit introduces new names for the handler options and updates the documentation, in order to address code review comments.

The mutex was only needed because s.View() was being accessed by a timer goroutine.

…ntext

This indirect approach causes confusion IMO.

This is not used and not documented; we can add this back if we find it will be useful in the future.

This aims to benchmark the event loop's processEvent method. ```sh % go test -v -run x -bench BenchmarkEventLoopWithUnsafe -count=20 -benchmem goos: darwin goarch: arm64 pkg: github.com/relab/hotstuff/eventloop BenchmarkEventLoopWithUnsafeRunInAddEventHandlers BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 347619 3456 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 344972 3444 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 348097 3422 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 349779 3452 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 343992 3464 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 345339 3426 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 347434 3424 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 342284 3478 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 346063 3451 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 350206 3426 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 347432 3377 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 360820 3313 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 360350 3324 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 359367 3320 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 361963 3314 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 355348 3323 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 362910 3417 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 347767 3456 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 345124 3447 ns/op 480 B/op 20 allocs/op BenchmarkEventLoopWithUnsafeRunInAddEventHandlers-12 347486 3424 ns/op 480 B/op 20 allocs/op PASS ok github.com/relab/hotstuff/eventloop 25.025s ```

johningve force-pushed the smartcontext branch 3 times, most recently from 6a18078 to 2ae3918 Compare August 23, 2022 06:44

johningve added 4 commits August 23, 2022 14:04

gorums race condition fix

1ea03eb

consensus: commit & advance view before voting

d2b33f3

If we are going to use a view context for voting, then we need to be in the correct view before we start the voting process. Otherwise, the vote could be canceled by the following view change.

synchronizer: smart view contexts

d8af9a1

This decouples contexts from the synchronizer and makes them listen for events on the event loop.

eventloop: more flexible handlers

469ebae

This commit aims to make the eventloop more flexible by allowing both async and sync handlers again. Handlers can be made async by passing a RunAsync handler option. Furthermore, observers have been replaced by a WithPriority handler option.

johningve force-pushed the smartcontext branch from 2ae3918 to 318c166 Compare August 23, 2022 12:15

johningve added 4 commits August 24, 2022 10:53

Timeout contexts

0c58196

Smart contexts that listen for timeouts

smart context tests

5234e44

eventloop: don't run funcs async

a9a70bf

eventloop: improve handler options API

c58cb4f

Now options are set by calling a WithOptions method on which RegisterHandler can be called afterwards.

johningve force-pushed the smartcontext branch from ddf8d8a to c58cb4f Compare August 24, 2022 08:53

johningve changed the title ~~Smartcontext~~ Implement smarter, event-based contexts for view changes and timeouts. Aug 24, 2022

johningve changed the title ~~Implement smarter, event-based contexts for view changes and timeouts.~~ Implement smarter, event-based contexts for view changes and timeouts Aug 24, 2022

johningve changed the title ~~Implement smarter, event-based contexts for view changes and timeouts~~ Implement event-based contexts for view changes and timeouts Aug 24, 2022

johningve marked this pull request as ready for review August 24, 2022 09:04

johningve requested a review from meling August 24, 2022 09:04

johningve and others added 3 commits April 25, 2023 16:53

Merge branch 'master' into smartcontext

50b46f1

Fixes lint issue

8147a7b

copylocks: <field> passes lock by value: github.com/relab/hotstuff/modules.Options contains sync.Mutex (govet)

meling requested changes May 4, 2023

View reviewed changes

meling and others added 7 commits May 4, 2023 15:37

Merge branch 'master' into smartcontext

829ac4d

gorums race condition fix

ed7c7e7

consensus: commit & advance view before voting

3c3fa91

If we are going to use a view context for voting, then we need to be in the correct view before we start the voting process. Otherwise, the vote could be canceled by the following view change.

synchronizer: smart view contexts

16a7c89

This decouples contexts from the synchronizer and makes them listen for events on the event loop.

eventloop: more flexible handlers

7739ff1

This commit aims to make the eventloop more flexible by allowing both async and sync handlers again. Handlers can be made async by passing a RunAsync handler option. Furthermore, observers have been replaced by a WithPriority handler option.

Timeout contexts

a3d073b

Smart contexts that listen for timeouts

smart context tests

d627714

johningve and others added 5 commits May 10, 2023 17:32

eventloop: don't run funcs async

bc79ebc

eventloop: improve handler options API

97b0ec8

Now options are set by calling a WithOptions method on which RegisterHandler can be called afterwards.

Fixes lint issue

7d569e3

copylocks: <field> passes lock by value: github.com/relab/hotstuff/modules.Options contains sync.Mutex (govet)

eventlop: Rename Handler Options and update documentation

1c68ca4

This commit introduces new names for the handler options and updates the documentation, in order to address code review comments.

johningve force-pushed the smartcontext branch from 829ac4d to 1c68ca4 Compare May 10, 2023 15:39

johningve and others added 7 commits May 10, 2023 18:09

synchronizer: remove need for mutex

7776a29

The mutex was only needed because s.View() was being accessed by a timer goroutine.

Merge branch 'smartcontext' of github.com:relab/hotstuff into smartco…

14a24b9

…ntext

Merge branch 'master' into smartcontext

77f3cf9

Removed extra processEvent in AddEvent due to bad merge

ba6e973

Remove the RegisterHandler and HandlerOptions indirection

da0424a

This indirect approach causes confusion IMO.

Removed direct func execution support from processEvent

0e3ef77

This is not used and not documented; we can add this back if we find it will be useful in the future.

meling approved these changes May 12, 2023

View reviewed changes

johningve merged commit 356f331 into master May 13, 2023

johningve deleted the smartcontext branch May 13, 2023 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Implement event-based contexts for view changes and timeouts #74

Implement event-based contexts for view changes and timeouts #74

Uh oh!

johningve commented Aug 19, 2022 •

edited

Loading

Uh oh!

codecov-commenter commented May 3, 2023 •

edited

Loading

Uh oh!

meling left a comment

Uh oh!

meling May 3, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

meling May 3, 2023

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Implement event-based contexts for view changes and timeouts #74

Implement event-based contexts for view changes and timeouts #74

Uh oh!

Conversation

johningve commented Aug 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented May 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

meling left a comment

Choose a reason for hiding this comment

Uh oh!

meling May 3, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

meling May 3, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

johningve commented Aug 19, 2022 •

edited

Loading

codecov-commenter commented May 3, 2023 •

edited

Loading