Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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(AP-6075): Add publisher & consumer level message deduplication #236
base: main
Are you sure you want to change the base?
feat(AP-6075): Add publisher & consumer level message deduplication #236
Changes from 9 commits
e545a98
7858855
68266a6
5c7bd5e
47d4e9b
56964a6
972e853
c770cd8
d0e433f
5693616
446ba48
1344903
762f138
876498c
bbc159e
9757cbb
8a68ad3
ed5f470
19494ae
391bc7b
fae22c5
1740b80
982e1e1
745b7b1
7a0da21
7bd8c86
abd05d2
a67b456
a122e01
31f5b28
49d81ee
cf729c7
14bfc7c
a02b53f
4a5dbed
d43e827
7aada4d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't redis client by itself support keyprefix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or is the idea here that we may want to have different prefixes on the same connection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking that we could use the same Redis connection as part of a service for both "regular" operations (like tmp storage management for different types of needs) and deduplication. In such a case, I think we need to handle key prefixing on dthe edup store. Feel free to disagree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need a user-defined key for this? wonder if there could be a default prefix per use-case, with a possibility for user to override
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep in mind that prefix is optional, I can add a default one in case user doesn't specify it (but in such case maybe they don't need prefixes at all? 🤔)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's OK to have prefix, but then let's explain what it's supposed to be and when different values need to be passed, so that user is not confused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the description to
RedisMessageDeduplicationStoreConfig