Flush Immediately After Publish Fix

[scottf]

Make sure bytes are written to data port when flushImmediatelyAfterPublish

[scottf]

@claude please review

[synadia-claude-reviewer]

Claude finished @scottf's task in 1m 23s —— View job

---

Code Review

• Gather context
• Read changed file and diff
• Analyze the fix
• Post review feedback

---

The fix is correct and well-targeted. This is a clear bug fix with minimal scope.

Bug Summary

In sendMessageBatch (NatsConnectionWriter.java), when msg.flushImmediatelyAfterPublish was true, the original code called dataPort.flush() without first calling dataPort.write(). The message bytes had been accumulated into sendBuffer but were never actually pushed to the dataPort's output stream — so the flush had no effect on the message just processed.

Fix Analysis

Before:

stats.incrementOut(size);
if (msg.flushImmediatelyAfterPublish) {
    dataPort.flush();  // ← nothing was written to dataPort yet!
}

After:

if (msg.flushImmediatelyAfterPublish) {
    dataPort.write(sendBuffer, sendPosition);  // ← write accumulated bytes first
    dataPort.flush();
    stats.registerWrite(sendPosition);
    sendPosition = 0;  // ← reset so end-of-loop write doesn't double-send
}
stats.incrementOut(size);

All four changes are correct and necessary:

1. dataPort.write(sendBuffer, sendPosition) — The missing write; this is the core of the fix.
2. dataPort.flush() — Now meaningful since bytes were just written.
3. stats.registerWrite(sendPosition) — Records the bytes written in this flush (may include bytes from earlier messages in the batch that preceded this flush-immediate message, which is correct behavior).
4. sendPosition = 0 — Prevents the final if (sendPosition > 0) write at the end of the method from double-writing the same bytes.

Notes

• The WriterMessageQueue.accumulate() loop (line 142) stops accumulating once it encounters a flushImmediatelyAfterPublish message as the cursor, but it can still include one earlier non-flush message in the batch. The fix correctly handles this by flushing all pending bytes at that point.
• The added comment (// if last message was flushImmediatelyAfterPublish, send position will always be 0) is accurate and helpful.

Verdict

LGTM. The fix is minimal, correct, and doesn't touch anything beyond the bug. No concerns.

[mtmk]

LGTM