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.

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

Tests: consumers with start sequence offset + ACK deleted messages #5781

Merged
merged 2 commits into from
Sep 6, 2024
Merged

Tests: consumers with start sequence offset + ACK deleted messages #5781

merged 2 commits into from
Sep 6, 2024

Conversation

mprimi
Copy link
Contributor

@mprimi mprimi commented Aug 12, 2024 •
edited
Loading

  1. Add test for consuming a stream with a given sequence start offset.
  • Durable consumer
  • Durable consumer with Bind
  • Pull consumer
  • Synchronous consumer
  • Callback subscription
  • Channel subscription
  • Get stream message
  • Get last stream message by subject
  1. Add test where a durable consumer ACKs messages that have been deleted from a stream

Signed-off-by: Marco Primi [email protected]

@mprimi mprimi force-pushed the marco/consumer-start-sequence-test branch from d8b811d to 11dcba1 Compare August 12, 2024 20:28
@mprimi mprimi marked this pull request as ready for review August 12, 2024 20:43
@mprimi mprimi requested a review from a team as a code owner August 12, 2024 20:43
@mprimi
Copy link
Contributor Author

mprimi commented Aug 12, 2024

Addressed review feedback. Travis errors are unrelated.

@mprimi mprimi force-pushed the marco/consumer-start-sequence-test branch 3 times, most recently from b26349f to d3cc8f8 Compare August 13, 2024 16:19
@mprimi
Copy link
Contributor Author

mprimi commented Aug 13, 2024 •
edited
Loading

Updated with additional test cases now that #5785 fixed them

--- PASS: TestJetStreamClusterConsumeWithStartSequence (2.76s)
    --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:1,Replicas:1 (0.02s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:1,Replicas:1/DurableConsumer (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:1,Replicas:1/DurableConsumerWithBind (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:1,Replicas:1/PreCreatedDurableConsumerWithBind (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:1,Replicas:1/PullConsumer (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:1,Replicas:1/PreCreatedPullConsumer (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:1,Replicas:1/SynchronousConsumer (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:1,Replicas:1/CallbackSubscribe (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:1,Replicas:1/ChannelSubscribe (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:1,Replicas:1/GetRawStreamMessage (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:1,Replicas:1/GetLastMessageBySubject (0.00s)
    --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:3,Replicas:3 (2.74s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:3,Replicas:3/DurableConsumer (0.61s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:3,Replicas:3/DurableConsumerWithBind (0.76s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:3,Replicas:3/PreCreatedDurableConsumerWithBind (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:3,Replicas:3/PullConsumer (0.01s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:3,Replicas:3/PreCreatedPullConsumer (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:3,Replicas:3/SynchronousConsumer (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:3,Replicas:3/CallbackSubscribe (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:3,Replicas:3/ChannelSubscribe (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:3,Replicas:3/GetRawStreamMessage (0.00s)
        --- PASS: TestJetStreamClusterConsumeWithStartSequence/Nodes:3,Replicas:3/GetLastMessageBySubject (0.00s)

@derekcollison
Copy link
Member

@neilalexander or @wallyqs let me know if this is good to go and should be merged.

neilalexander
neilalexander reviewed Aug 13, 2024
View reviewed changes
Copy link
Member

@neilalexander neilalexander left a comment

Choose a reason for hiding this comment

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

Overall tests look OK, but let's clean up the error handling in the defers, as using the require_ functions is much more brief.

server/jetstream_cluster_4_test.go Outdated Show resolved Hide resolved
server/jetstream_cluster_4_test.go Outdated Show resolved Hide resolved
@mprimi mprimi force-pushed the marco/consumer-start-sequence-test branch from d3cc8f8 to 6c6ceb4 Compare August 13, 2024 20:12
@mprimi mprimi changed the title Test variations of consumer with start sequence offset Tests: consumers with start sequence offset + ACK deleted messages Aug 13, 2024
@mprimi
Copy link
Contributor Author

mprimi commented Aug 13, 2024

@neilalexander Addressed feedback, thank you.

While this is open, I added a second commit with a new test (ACK of deleted messages)

@mprimi mprimi force-pushed the marco/consumer-start-sequence-test branch from 6c6ceb4 to 0824883 Compare August 16, 2024 17:54
@mprimi
Consumer test with stream start sequence
e952a3a 
Test various ways of consuming a stream with a given sequence number offset.
@mprimi
Test ACKing deleted messages
11b2bd7 
Test that explicitly ACKs messages that have been already removed from a stream
@mprimi mprimi force-pushed the marco/consumer-start-sequence-test branch from 0824883 to 11b2bd7 Compare September 5, 2024 22:46
derekcollison
derekcollison approved these changes Sep 6, 2024
View reviewed changes
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

@derekcollison derekcollison merged commit 48ad100 into nats-io:main Sep 6, 2024
2 checks passed
@wallyqs wallyqs mentioned this pull request Sep 17, 2024
Merged
wallyqs pushed a commit that referenced this pull request Sep 17, 2024
@mprimi @wallyqs
Tests: consumers with start sequence offset + ACK deleted messages (#…
93a62aa 
…5781)

1. Add test for consuming a stream with a given sequence start offset.

- Durable consumer
- Durable consumer with `Bind`
- Pull consumer
- Synchronous consumer
- Callback subscription
- Channel subscription
- Get stream message
- Get last stream message by subject

2. Add test where a durable consumer ACKs messages that have been
deleted from a stream

Signed-off-by: Marco Primi <[email protected]>
wallyqs added a commit that referenced this pull request Sep 17, 2024
@wallyqs
Cherry picks for v2.10.21 RC1 (#5898)
032ac9c 
Includes:

- #5854
- #5863
- #5869
- #5886
- #5893
- #5896
- #5852
- #5781
- #5887
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derekcollison derekcollison derekcollison approved these changes

@wallyqs wallyqs Awaiting requested review from wallyqs

@neilalexander neilalexander Awaiting requested review from neilalexander

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mprimi @derekcollison @wallyqs @neilalexander