Event stream MVP #9013

drewbailey · 2020-10-02T18:38:19Z

This PR adds an event streaming to Nomad.

Event Types

Currently there are event types for the major Nomad objects, JobEvent, AllocEvent, EvalEvent, DeploymentEvent and NodeEvent. These events all contain a fully hydrated object for the event.

Event Creation

Events are sourced through FSM requests. During the request a transactions changes are tracked and used to create events.

/v1/event/stream

A new /v1/event/stream api is added to consume events from. The output will be streamed as new line delimted JSON (ndjson.org). An example request to stream all deployment events would be

curl -G localhost:4646/v1/events/stream \
 --data-urlencode "topic=Deployment:*"

More API documentation to follow in a separate PR.

Durability

Currently events are held in memory for 1 hour and the configuration option for how many to store is hardcoded to 250. Eventually a configurable amount of messages to hold in memory and a configurable amount of messages to store in go-memdb for durability will be exposed.

drewbailey · 2020-10-02T20:16:41Z

Things I am tracking and need to be resolved:

schmichael

I'll finish up on Monday. Looking really great so far!

api/event.go

schmichael · 2020-10-02T22:09:57Z

nomad/event_endpoint.go

+	// ACL check
+	// TODO(drew) ACL checks need to be per topic
+	// All Events        Management
+	// System Events     Management
+	// Node Events       NamespaceCapabilityReadEvents
+	// Job/Alloc Events  NamespaceCapabilityReadEvents
+	if aclObj, err := e.srv.ResolveToken(args.AuthToken); err != nil {
+		handleJsonResultError(err, nil, encoder)
+		return
+	} else if aclObj != nil && !aclObj.IsManagement() {


If you don't want to refine this before merging to master just make sure to file an issue to track the future work.

schmichael · 2020-10-02T22:16:28Z

nomad/event_endpoint.go

+			return
+		}
+		select {
+		case <-errCh:


Why are we receiving on the errCh here?

I know this is the pattern used in Monitor, but is checking the error returned by conn.Read above even necessary? Seems like in streaming APIs where we never expect to read anything, conn.Read returning anything at all is an indication we should shut things down?

I don't think we need receive on errCh here, removing. I think we want to try to read from the conn just to make sure the conneciton hasn't closed?

nomad/event_endpoint.go

nomad/fsm.go

vercel · 2020-10-06T13:17:48Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hashicorp/nomad/mo67ibodj
✅ Preview: https://nomad-git-event-stream.hashicorp.vercel.app

shoenig · 2020-10-06T15:20:28Z

nomad/fsm.go

-	// }
+	if n.config.EnableEventPublisher {
+		if err := rehydratePublisherFromState(n.state); err != nil {
+			n.logger.Error("Error re-hydrating event publisher during restore", "error", err)


no error return?

Event durability seemed like a best effort, failing the entire snapshot restore process because we couldn't rehydrate seemed aggressive, but maybe its worth doing? Maybe this needs a TODO for when it comes out of beta? not really sure

Soft failing on event restoration seems like the right tradeoff to me. My only fear is that it may be very difficult to know that durability works at all! I'm not sure our E2E suite currently supports restarting servers as part of a test, and users may never notice this log line and be unaware there's some functionality missing. A unit test should be sufficient to ensure durability is working. Unit testing and even manual testing the soft fail case seems very awkward since you would have to manipulate the snapshot very carefully.

schmichael

Still haven't finished. Sorry. event_buffer is tricky stuff.

command/agent/config.go

command/agent/config_parse_test.go

schmichael · 2020-10-05T16:01:27Z

nomad/event_endpoint.go

+	// TODO(drew) handle streams without ACLS
+	reqToken := args.AuthToken
+	if reqToken == "" {
+		// generate a random request token
+		reqToken = uuid.Generate()


Not sure I understand what's not being handled here. What's the request token represent? It can't be a unique consumer identifier because a single auth token could open multiple streams.

If TODOs represent missing core functionality, please file an issue for tracking before merging.

thought I replied here but I added a comment to the file.. if an auth token is passed, we want to to add it to the subscription request so the event publisher can track ACL events and force close subscriptions if a token expires.

If ACLs are disabled we give it a random UUID, to satisfy the subscription map, but it will never be closed due to token expiry

schmichael · 2020-10-05T22:23:24Z

nomad/state/apply_plan_events.go

+)
+
+func ApplyPlanResultEventsFromChanges(tx ReadTxn, changes Changes) (structs.Events, error) {
+	var events []structs.Event


Might be nice to avoid reallocations from appending to the slice in this critical path.

Suggested change

var events []structs.Event

var events []stream.Event

events := make([]stream.Event, 0, len(changes))

len(changes) won't always end up being equal to the events that are created, (no new events for changes to index table)

nomad/state/apply_plan_events.go

nomad/stream/event_buffer.go

schmichael · 2020-10-06T22:42:44Z

nomad/stream/event_buffer.go

+	// Check if the reader is too slow and the event buffer as discarded the event
+	select {
+	case <-i.link.droppedCh:
+		return nil, fmt.Errorf("event dropped from buffer")


Why isn't this checked in the select above?

If droppedCh is ready linkCh will likely be as well (slow reader is on event 1, the new head is event 3, so 1 linkCh to 2 will be ready). Since both will be ready the select will be random so we need to check after, I'll add a comment.

nomad/stream/event_buffer.go

schmichael · 2020-10-06T22:49:11Z

nomad/stream/event_buffer.go

+	b.tail.Store(item)
+
+	// Increment the buffer size
+	size := atomic.AddInt64(b.size, 1)


I think this is only ever used by externally synchronized methods and therefore doesn't need synchronization? Not a big deal to keep just to avoid races in the future.

nomad/stream/event_buffer.go

* only enable publisher based on config * add default prune tick * back out state abandon changes on fsm close

* 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

This Commit adds an /v1/events/stream endpoint to stream events from. The stream framer has been updated to include a SendFull method which does not fragment the data between multiple frames. This essentially treats the stream framer as a envelope to adhere to the stream framer interface in the UI. If the `encode` query parameter is omitted events will be streamed as newline delimted JSON.

* Node Drain events and Node Events (#8980) Deployment status updates handle deployment status updates (paused, failed, resume) deployment alloc health generate events from apply plan result txn err check, slim down deployment event one ndjson line per index * consolidate down to node event + type * fix UpdateDeploymentAllocHealth test invocations * fix test

* generic eval update event first pass at alloc client update events * api/event client

table for durability

address pr feedback

are removed from the event buffer. Wire up event buffer size config, use pointers for structs.Events instead of copying.

properly wire up durable event count move newline responsibility moves newline creation from NDJson to the http handler, json stream only encodes and sends now ignore snapshot restore if broker is disabled enable dev mode to access event steam without acl use mapping instead of switch use pointers for config sizes, remove unused ttl, simplify closed conn logic

github-actions · 2022-12-15T02:18:30Z

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

drewbailey requested review from shoenig, notnoop and schmichael October 2, 2020 18:38

schmichael reviewed Oct 2, 2020

View reviewed changes

drewbailey force-pushed the event-stream branch 2 times, most recently from 0ed92a6 to c12d912 Compare October 6, 2020 13:17

vercel bot deployed to Preview October 6, 2020 13:17 View deployment

vercel bot deployed to Preview October 6, 2020 13:48 View deployment

vercel bot deployed to Preview October 6, 2020 15:08 View deployment

shoenig reviewed Oct 6, 2020

View reviewed changes

shoenig approved these changes Oct 6, 2020

View reviewed changes

shoenig self-requested a review October 6, 2020 15:22

vercel bot deployed to Preview October 6, 2020 17:46 View deployment

schmichael reviewed Oct 6, 2020

View reviewed changes

drewbailey force-pushed the event-stream branch from b8a0cbd to 9925e7d Compare October 7, 2020 14:42

vercel bot temporarily deployed to Preview October 7, 2020 14:42 Inactive

vercel bot deployed to Preview October 7, 2020 14:43 View deployment

vercel bot deployed to Preview October 7, 2020 15:47 View deployment

vercel bot deployed to Preview October 7, 2020 16:48 View deployment

vercel bot deployed to Preview October 7, 2020 17:15 View deployment

vercel bot deployed to Preview October 7, 2020 17:44 View deployment

vercel bot temporarily deployed to Preview October 7, 2020 18:39 Inactive

vercel bot deployed to Preview October 7, 2020 18:46 View deployment

vercel bot deployed to Preview October 7, 2020 21:16 View deployment

vercel bot deployed to Preview October 8, 2020 12:31 View deployment

drewbailey force-pushed the event-stream branch from 364b23e to 17ebff0 Compare October 8, 2020 14:14

vercel bot deployed to Preview October 8, 2020 14:15 View deployment

vercel bot deployed to Preview October 8, 2020 14:25 View deployment

drewbailey added 15 commits October 14, 2020 12:44

Events/cfg enable publisher (#8916)

2626309

* only enable publisher based on config * add default prune tick * back out state abandon changes on fsm close

Events/event source node (#8918)

8bd15b5

* 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

Events/eval alloc events (#9012)

4f97bf8

* generic eval update event first pass at alloc client update events * api/event client

api comments

8a57ee8

wire up enable_event_publisher

e7e6df3

writetxn can return error, add alloc and job generic events. Add events

47d1a33

table for durability

use Events to wrap index and events, store in events table

004f634

event durability count and cfg

8a3130a

go-memdb v1.3.0

66bda44

rehydrate event publisher on snapshot restore

1288b18

address pr feedback

Add EvictCallbackFn to handle removing entries from go-memdb when they

39ef326

are removed from the event buffer. Wire up event buffer size config, use pointers for structs.Events instead of copying.

handle txn returning error

8711376

namespace filtering

7102820

drewbailey force-pushed the event-stream branch from 4b64007 to 8c88f29 Compare October 14, 2020 16:45

vercel bot deployed to Preview October 14, 2020 16:45 View deployment

vercel bot deployed to Preview October 14, 2020 17:55 View deployment

drewbailey force-pushed the event-stream branch from 092918a to 3c15f41 Compare October 14, 2020 18:15

vercel bot deployed to Preview October 14, 2020 18:15 View deployment

vercel bot deployed to Preview October 14, 2020 18:21 View deployment

event stream changelog

8c5f3a0

drewbailey force-pushed the event-stream branch from 8dc90ba to 8c5f3a0 Compare October 14, 2020 18:28

vercel bot deployed to Preview October 14, 2020 18:28 View deployment

drewbailey merged commit d68e3d4 into master Oct 14, 2020

drewbailey deleted the event-stream branch October 14, 2020 18:45

drewbailey mentioned this pull request Oct 16, 2020

Events/msgtype cleanup #9117

Merged

github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Event stream MVP #9013

Event stream MVP #9013

drewbailey commented Oct 2, 2020

drewbailey commented Oct 2, 2020 •

edited

Loading

schmichael left a comment

schmichael Oct 2, 2020

schmichael Oct 2, 2020

drewbailey Oct 6, 2020

vercel bot commented Oct 6, 2020 •

edited

Loading

shoenig Oct 6, 2020

drewbailey Oct 6, 2020 •

edited

Loading

schmichael Oct 6, 2020

schmichael left a comment

schmichael Oct 5, 2020

drewbailey Oct 9, 2020

schmichael Oct 5, 2020

drewbailey Oct 7, 2020

schmichael Oct 6, 2020

drewbailey Oct 6, 2020

schmichael Oct 6, 2020

github-actions bot commented Dec 15, 2022

	var events []structs.Event
	var events []stream.Event
	events := make([]stream.Event, 0, len(changes))

Event stream MVP #9013

Event stream MVP #9013

Conversation

drewbailey commented Oct 2, 2020

Event Types

Event Creation

/v1/event/stream

Durability

drewbailey commented Oct 2, 2020 • edited Loading

schmichael left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Oct 6, 2020 • edited Loading

Choose a reason for hiding this comment

drewbailey Oct 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schmichael left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 15, 2022

drewbailey commented Oct 2, 2020 •

edited

Loading

vercel bot commented Oct 6, 2020 •

edited

Loading

drewbailey Oct 6, 2020 •

edited

Loading