Skip to content

Fix ValidateCounterId so an ID exactly equal to MaxCounterId also fails - #143

Merged
mikeb01 merged 1 commit into
masterfrom
ebowden/agrona-counters-validate-counter-id
May 18, 2026
Merged

mikeb01 merged 1 commit into
masterfrom
ebowden/agrona-counters-validate-counter-id

Conversation

@strangelydim

Copy link
Copy Markdown
Contributor

NOTE: This PR being created as a draft for discussion.

Java's maxCounterId is set to:

this.maxCounterId = (valuesBuffer.capacity() / COUNTER_LENGTH) - 1;

whereas .NET's is:

MaxCounterId = valuesBuffer.Capacity / COUNTER_LENGTH;

The better fix, IMO, is to change .NET to match Java, so "max counter ID" really is the max counter ID, not one higher than the max ID.

However, it's a public value, and users might already have code that uses it, so that would be a potentially breaking change.

The change in this PR is the simpler, non-breaking thing we could do. Which do we prefer?

@strangelydim
strangelydim requested a review from JPWatson May 1, 2026 23:55
@mikeb01

mikeb01 commented May 11, 2026

Copy link
Copy Markdown
Contributor

I think the proper change "should" be safe. The only real use case for this is using it as a bound. If any users are correctly doing <= MaxCounterId, then they will see the bound being 1 lower than before, i.e. more conservative. As long as we include it in the release notes I think making it consistent is important.

@strangelydim
strangelydim marked this pull request as ready for review May 15, 2026 19:09
@mikeb01
mikeb01 merged commit c57ee57 into master May 18, 2026
4 checks passed
@mikeb01
mikeb01 deleted the ebowden/agrona-counters-validate-counter-id branch May 18, 2026 21:53
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