Skip to content

Release JetStream.Publisher 1.0.0-preview.7#63

Merged
mtmk merged 1 commit into
mainfrom
release-jetstream-publisher-1.0.0-preview.7
May 8, 2026
Merged

Release JetStream.Publisher 1.0.0-preview.7#63
mtmk merged 1 commit into
mainfrom
release-jetstream-publisher-1.0.0-preview.7

Conversation

@mtmk
Copy link
Copy Markdown
Collaborator

@mtmk mtmk commented May 8, 2026

Preview release of JetStream.Publisher adding end-of-batch sentinel support to NatsJSBatchPublisher. Requires NATS Server 2.14+ for the new CloseAsync path.

@synadia-claude-reviewer
Copy link
Copy Markdown

synadia-claude-reviewer Bot commented May 8, 2026

Claude finished @mtmk's task in 1m 43s —— View job


PR Review: Release JetStream.Publisher 1.0.0-preview.7

  • Gather context: examine PR diff and relevant source files
  • Review version change
  • Review the batch publisher changes referenced in the PR
  • Post review summary

Summary

The PR itself is a single-line version bump (1.0.0-preview.61.0.0-preview.7). The substantive change is in the referenced commit (e19e066), which added CloseAsync — an end-of-batch (EOB) sentinel path to NatsJSBatchPublisher.

CloseAsync / EOB Implementation

Correctness looks good:

  • The _sequence counter is intentionally not incremented for the EOB sentinel (line 279–281 in NatsJSBatchPublisher.cs), so Size stays consistent with ack.BatchSize. The expectedSize validation at line 325 (currentSeq - 1 for EOB) correctly checks this invariant.
  • _closed = true is set inside CommitMsgInternalAsync's lock section (line 275) before any network I/O, so concurrent CloseAsync calls are safe: the loser sees _closed = true and throws NatsJSBatchClosedException.
  • Empty-batch guard (_sequence == 0 || _batchSubject == null) is defensive and correct; the failed CloseAsync leaves the batch open for subsequent use as verified by the test at line 646–650.
  • The Nats-Batch-Commit: eob header value is clearly documented in NatsJSBatchHeaders.BatchCommit.

One minor observation:

CloseAsync sends the sentinel on _batchSubject (the subject of message 1). For multi-subject batches the interface docs note this explicitly, but callers using entirely different subjects for every message might find it unexpected. This is a documentation/design trade-off rather than a bug, and the current docs cover it.

Test coverage is thorough: basic close, Size == ack.BatchSize consistency, empty-batch guard, close-after-close, and close-after-commit are all exercised with server-version guards for the 2.14+ EOB path.

Version Bump

version.txt change is correct and consistent with a preview release following the feature merge.

No blocking issues. The implementation is sound.

@mtmk mtmk merged commit 66fb011 into main May 8, 2026
10 checks passed
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.

1 participant