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

feat(wadm)!: Switch wadm events to limits-based stream and create a new sourcing stream for EventConsumer #310

Conversation

joonas
Copy link
Member

@joonas joonas commented Jun 30, 2024

Feature or Problem

This switches from our custom mirroring code to NATS Stream Sourcing.

It also changes the wadm events to follow the pattern established for wasmbus events where they are now published under wadm.evt.<lattice-id>.<event-type> to make the sourcing consistent.

Finally, it also includes code to clean up any existing wadm_(multitenant)_mirror streams on startup so that we don't end up with conflicting subjects (which would prevent the new streams from being created in the first place).

This effectively accomplishes the same thing as #295 using slightly different strategy, while also making the wadm_events (and thus turning wadm.evt.<lattice-id>.<event-type> into a stream-based API).

Related Issues

Release Information

Consumer Impact

Testing

Unit Test(s)

Acceptance or Integration

Manual Verification

@joonas joonas force-pushed the feat/use-nats-stream-sourcing-for-event_consumer branch from 23ed0a5 to ba07c23 Compare June 30, 2024 00:40
@joonas joonas changed the title feat(wadm): Switch wadm events to limits-based stream and create a new sourcing stream for EventConsumer feat(wadm)!: Switch wadm events to limits-based stream and create a new sourcing stream for EventConsumer Jun 30, 2024
src/nats.rs Show resolved Hide resolved
return Ok(stream);
} else {
warn!("Found stream {name} with different configuration, deleting and recreating");
context.delete_stream(name).await?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this going to cause issues if someone has created their own stream with their requirements (such as more replicas or other things) or if something else is already consuming these downstream? I'd rather just sort the list of subjects and compare that way if possible

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this going to cause issues if someone has created their own stream with their requirements (such as more replicas or other things) or if something else is already consuming these downstream?

For downstream this will not create issues, I tested this locally and the net-net on this is that once the Stream is re-created, NATS will continue where it left off, meaning that the only messages that could be lost are the ones that are in the stream exactly at the moment when it's being re-created.

As for letting someone (pre-)create this stream but with different configuration, I'm not sure how easy that would be to support. It feels like if that's the path we want to go down, a better strategy would be to explicitly expose the knobs for configuring the various parts of these stream as flags/environment variables that could be passed down to code that's (re)creating the streams.

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

We already let people specify buckets, so letting them specify streams seems ok. My principal concern is with the stream capturing all the wasmbus.evt stuff since that is the one I consider most likely an advanced wasmCloud user might want to configure to have more retention or replicas. This is fine for now though

src/nats.rs Outdated Show resolved Hide resolved
src/nats.rs Show resolved Hide resolved
@joonas joonas force-pushed the feat/use-nats-stream-sourcing-for-event_consumer branch 2 times, most recently from 871190f to 8903177 Compare July 1, 2024 22:24
…ew sourcing stream for EventConsumer

Signed-off-by: Joonas Bergius <joonas@cosmonic.com>
@joonas joonas force-pushed the feat/use-nats-stream-sourcing-for-event_consumer branch from 8903177 to 1aecf67 Compare July 1, 2024 23:46
@thomastaylor312 thomastaylor312 merged commit 8bdaba7 into wasmCloud:main Jul 2, 2024
3 checks passed
@joonas joonas deleted the feat/use-nats-stream-sourcing-for-event_consumer branch July 2, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants