(2.14) [FIXED] Filestore lost tombstones#7387
Merged
neilalexander merged 1 commit intomainfrom Nov 13, 2025
Merged
Conversation
Member
|
Be good to chase the tombstones as well, since when a prior block is compacted then deleted messages can no longer reappear and we could delete the tombstone at that point as well. |
Member
Author
Yep, that's done now as well. When |
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
MauriceVanVeen
force-pushed
the
maurice/fs-lost-tombstones
branch
from
91e85a3 to
0cb363b
Compare
neilalexander
added a commit
that referenced
this pull request
Nov 18, 2025
Fixes a data race introduced in #7387: ``` WARNING: DATA RACE Read at 0x00c0002d17f0 by goroutine 527: github.com/nats-io/nats-server/v2/server/avl.(*SequenceSet).Range() server/avl/seqset.go:125 +0x3b github.com/nats-io/nats-server/v2/server.(*fileStore).deleteMap() server/filestore.go:10676 +0x164 Previous write at 0x00c0002d17f0 by goroutine 182: github.com/nats-io/nats-server/v2/server/avl.(*SequenceSet).Insert() server/avl/seqset.go:45 +0x6d github.com/nats-io/nats-server/v2/server.(*fileStore).PurgeEx() server/filestore.go:8626 +0x1b0c ``` Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Given two message blocks, the first containing 100 messages, the second containing 99 tombstones for the messages in the first block, except for the first message/seq. When a new message was written into the second block and removed. The filestore would recognize this block as empty and then remove it. However, if a server would be ungracefully restarted all the 99 removed messages would re-appear, since the tombstones for them were removed when removing the second block.
This PR fixes this issue by recognizing when a block contains tombstones for prior blocks. In this case it will not remove a block until it is both empty and contains no tombstones for prior blocks.
Additionally,
fs.syncBlockscan now not only remove tombstones if they are below the first sequence, but also if a tombstone that's contained in a block is not actively referencing a message in another block. Which means that if a block contains a tombstone for a block that has been removed, then that tombstone can be compacted as well.fs.deleteBlocks()has also been improved since multiple empty blocks containing tombstones at various sequences would otherwise result in many sequential delete ranges. Those are now intelligently collapsed into one large delete range spanning all these blocks.This is reserved for 2.14 as it requires the compatibility of below PR, and could result in first sequences not moving up if a downgrade would be performed and there would be empty blocks with tombstones prior to the downgrade.
Follow-up of #7384
Signed-off-by: Maurice van Veen github@mauricevanveen.com