-
Notifications
You must be signed in to change notification settings - Fork 44
Fixed critical seqNo renumbering bug in TopicWriter #545
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
base: main
Are you sure you want to change the base?
Conversation
5205961 to
a4e6fbf
Compare
|
If the Instead, we could return an opaque object (e.g. a interface SeqToken {
resolveSeqNo(): bigint
}
class TopicWriter {
write(
data: Uint8Array,
extra?: {
seqNo?: bigint
createdAt?: Date
metadataItems?: Record<string, Uint8Array>
}
): SeqToken {
...
}
} |
I thought about it, but it's a loss of backward compatibility. I intentionally did not make the return value an object so that there would not be many references and the GC could dispose of memory more efficiently. |
- Add resolveSeqNo() method to get final seqNo for messages written before session initialization - Track seqNo shifts through SeqNoShiftEvent segments when session reconnects - Update write() JSDoc to warn about temporary seqNo values before session initialization - Implement efficient seqNo shift tracking using range merging and inversion algorithms - Update flush() documentation to clarify when seqNo values become final Breaking changes: None (backward compatible)
- Extract seqNo mapping logic into SeqNoResolver class for better testability - Add SeqNoShiftBuilder to accumulate shifts during message renumbering - Simplify manual seqNo mode: only adjust pointers, no array mutations - Add comprehensive unit tests for SeqNoResolver, SeqNoShiftBuilder, SeqNoManager - Improve code documentation and comments throughout updateWriteSession - Add integration test verifying resolveSeqNo works correctly
a4e6fbf to
3873268
Compare
- Fix seqNo renumbering bug: messages written before session initialization are now properly renumbered after receiving lastSeqNo from server - Remove return value from write() method (now returns void) to simplify API - Remove resolveSeqNo() method and related seqNo shift tracking infrastructure - Update tests to remove assertions on write() return values - Add changeset describing bug fix and API simplification
3917974 to
b0ee2e9
Compare
…tion - Remove lastSeqNo update from _flush() - it was updating before messages were actually sent to server - Fix renumbering logic in _on_init_response to properly handle messages written before init - Check if messages in buffer need renumbering by comparing their seqNo with serverLastSeqNo - Never renumber messages if user provided seqNo (manual mode) - Update lastSeqNo only when session is initialized or new message is written, not on ACKs
b0ee2e9 to
a33d8b1
Compare
What
Fixed critical seqNo renumbering bug in TopicWriter and simplified API by removing return value from
write()method.Why
Bug fix:
Previously, when messages were written before session initialization (before receiving
lastSeqNofrom server), auto-generated seqNo started from 0 and were never renumbered after session initialization. This caused seqNo conflicts when:producerIdThe fix ensures that all pending messages in auto seqNo mode are properly renumbered to continue from server's
lastSeqNo + 1after session initialization.API simplification:
The
write()method previously returned seqNo values that could be misleading - they were temporary before session initialization and could change after reconnection. Removing the return value simplifies the API and prevents misuse.Changes
Bug Fix: seqNo Renumbering
machine.ts: Implemented proper seqNo renumbering inupdateWriteSessionactionserverLastSeqNo + 1API Changes
writer.ts:write()method now returnsvoidinstead ofbigint(removed seqNo return value)isSessionInitializedflag to track session statewriter.sessionevent handling to usenextSeqNofrom state machineseqNoModeparameter towriter.writeevent for state machineCode Improvements
types.ts: UpdatedWriterEmittedtype to includenextSeqNoinwriter.sessioneventseqno-manager.ts: Enhanced to support mode detection and state trackingwrite()return values, verify seqNo renumbering works correctlyMigration Guide
If you were storing seqNo from
write()return value:// Before
let seqNo = writer.write(data)
await saveToDatabase(seqNo)
// After
writer.write(data)
let lastSeqNo = await writer.flush() // Get final seqNo after flush
await saveToDatabase(lastSeqNo)Important:
extra.seqNo) remain final and unchanged - no migration neededflush()completes, all seqNo values up to returnedlastSeqNoare finalwrite()calls to track individual messages if neededTesting
write())Files Changed
packages/topic/src/writer2/machine.ts- Implemented seqNo renumbering logicpackages/topic/src/writer2/writer.ts- Removed return value, added session trackingpackages/topic/src/writer2/types.ts- Updated event typespackages/topic/src/writer2/seqno-manager.ts- Enhanced seqNo managementpackages/topic/tests/writer2.test.ts- Updated tests.changeset/remove-write-seqno-return.md- Added changesetChecklist