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

KV never set duplicate window #577

Merged
merged 1 commit into from
Jul 24, 2024
Merged

KV never set duplicate window #577

merged 1 commit into from
Jul 24, 2024

Conversation

scottf
Copy link
Contributor

@scottf scottf commented Jul 24, 2024

No description provided.

@scottf scottf requested a review from mtmk July 24, 2024 15:27
Copy link
Collaborator

@mtmk mtmk left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -226,13 +226,8 @@ private static StreamConfig CreateStreamConfig(NatsKVConfig config)
// MirrorDirect =
// Mirror =
Retention = StreamConfigRetention.Limits, // from ADR-8
DuplicateWindow = config.MaxAge != default ? config.MaxAge : TimeSpan.FromMinutes(2), // 120_000_000_000ns, from ADR-8
Copy link
Collaborator

Choose a reason for hiding this comment

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

just to note bug was that when MaxAge was set to something like 30 minutes, DumplicateWindow was always set to the same value and that was creating a wrong config on the server. by letting the server decide we're avoiding any checks here. if we wanted to duplicate what Go client is doing, we could set DuplicateWindow to 2min here (avoiding the MaxAge check) and leave the check below. but I think this is a better solution. less complexity.

@scottf scottf merged commit 4832d15 into main Jul 24, 2024
13 checks passed
@scottf scottf deleted the kv-dont-set-max-age branch July 24, 2024 16:38
mtmk added a commit that referenced this pull request Jul 24, 2024
* Obj store empty list fixed (#578)
* KV never set duplicate window (#577)
* Resolved issue of ObjStore GetInfo MTime returning 0 (#567)
@mtmk mtmk mentioned this pull request Jul 24, 2024
mtmk added a commit that referenced this pull request Jul 24, 2024
* Obj store empty list fixed (#578)
* KV never set duplicate window (#577)
* Resolved issue of ObjStore GetInfo MTime returning 0 (#567)
mtmk added a commit that referenced this pull request Aug 2, 2024
* Handle various protocol errors and socket exceptions (#584)
* Fetch idle timeout default fix (#582)
* Obj store empty list fixed (#578)
* KV never set duplicate window (#577)
* Resolved issue of ObjStore GetInfo MTime returning 0 (#567)
* Add logging enhancements and improve error handling (#570)
* Base64 Encoding simplification + optimization (#549)
* Use string.Create when building a new inbox string (#551)
@mtmk mtmk mentioned this pull request Aug 2, 2024
mtmk added a commit that referenced this pull request Aug 2, 2024
* Handle various protocol errors and socket exceptions (#584)
* Fetch idle timeout default fix (#582)
* Obj store empty list fixed (#578)
* KV never set duplicate window (#577)
* Resolved issue of ObjStore GetInfo MTime returning 0 (#567)
* Add logging enhancements and improve error handling (#570)
* Base64 Encoding simplification + optimization (#549)
* Use string.Create when building a new inbox string (#551)
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