Events/event source node #8918
Events/event source node #8918
Conversation
drewbailey
force-pushed
the
event-stream
branch
from
5e9a9ff to
3cb5f5e
Compare
drewbailey
force-pushed
the
events/event-source-node
branch
from
6a86160 to
72a69c4
Compare
drewbailey
force-pushed
the
event-stream
branch
from
3cb5f5e to
2eadc8d
Compare
example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event
drewbailey
force-pushed
the
events/event-source-node
branch
from
7991cc2 to
24d8ef2
Compare
| } | ||
|
|
||
| // DeleteNode deregisters a batch of nodes | ||
| func (s *StateStore) DeleteNodeCtx(ctx context.Context, index uint64, nodes []string) error { |
There was a problem hiding this comment.
By splitting this method out into one version with ctx and the other without, we're not forcing all consumers of this state store function to update and thread their changes through the event store. That's maybe convenient for tests but seems like it could be a source of bugs later.
There was a problem hiding this comment.
Once these changes land in master, I was thinking DeleteNodeCtx would go away and DeleteNode or any other state store func would get a context as its first argument. I refrained from doing so here mainly because of tests, there are 252 invocations for UpsertNode in test files and since this PR is going into a long lived feature branch I just honestly didn't want to deal with all the potential merge conflicts 😅
If this route is agreed upon by everyone I'll go back and comment / TODO it being refactored away
There was a problem hiding this comment.
I totally missed this wasn't going to master, so 👍 on that approach.
| defer metrics.MeasureSince([]string{"nomad", "fsm", "register_node"}, time.Now()) | ||
| var req structs.NodeRegisterRequest | ||
| if err := structs.Decode(buf, &req); err != nil { | ||
| panic(fmt.Errorf("failed to decode request: %v", err)) | ||
| } | ||
|
|
||
| ctx := context.WithValue(context.Background(), state.CtxMsgType, reqType) |
There was a problem hiding this comment.
As far as I can tell there's a 1:1 mapping between message types and n.apply* functions. So we're creating a context and threading the message type from raft -> fsm -> state store. There's only ever 1 parameter and it's the message type, which the apply function already needs to know. What do we get by creating a context rather than tacking on the msgType as a new parameter lower down the stack?
|
I was wary of a In practice, state store methods that Write are only ever called from the FSM, but there are tons of invocations in tests and I wasn't sure about tacking on fake message types or making tests use proper message types when setting up tests vs just having a context.Background(). Context also felt open ended and could be used for tracing or log correlation down the road. |
|
merging into |
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
| s.StopEventPublisher() | ||
| close(s.abandonCh) | ||
| } | ||
|
|
||
| // StopStopEventPublisher calls the cancel func for the state stores event | ||
| // publisher. It should be called during server shutdown. | ||
| func (s *StateStore) StopEventPublisher() { | ||
| s.stopEventPublisher() | ||
| } | ||
|
|
There was a problem hiding this comment.
Why not just call Abandon in tests? Or perhaps I'm misunderstanding the point of this wrapper?
There was a problem hiding this comment.
StopEventPublisher will be used during server shutdown, the state store tests can call Abandon(), when I had Abandon() being called during fsm/server shutdown it caused weird leadership flapping https://app.circleci.com/pipelines/github/hashicorp/nomad/11813/workflows/611f362f-009a-497f-a0e2-60cc25977fb6/jobs/100908
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
* Node Register/Deregister event sourcing example upsert node with context fill in writetxnwithctx ctx passing to handle event type creation, wip test node deregistration event drop Node from registration event * node batch deregistration
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Source node registration and deregistration events from the FSM
This PR updates the FSM to pass in the message type to apply functions. A Context is added to state store methods to allow retrieving the msg type during event creation.
The bodies of the two events here, NodeRegisration and NodeDeregistration may change some, but depending on feedback to the FSM / state store this is how all events will be generated