Skip to content

Enable binary snapshots on initial route connections#7476

Closed
alexbozhenko wants to merge 2 commits intomainfrom
alex/snapshot_fix
Closed

Enable binary snapshots on initial route connections#7476
alexbozhenko wants to merge 2 commits intomainfrom
alex/snapshot_fix

Conversation

@alexbozhenko
Copy link
Copy Markdown
Member

Signed-off-by: Alex Bozhenko alexbozhenko@gmail.com

Signed-off-by: Alex Bozhenko <alexbozhenko@gmail.com>
Comment thread server/route.go
stats: nil,
offline: false,
js: info.JetStream,
binarySnapshots: true,
Copy link
Copy Markdown
Member Author

@alexbozhenko alexbozhenko Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was initially added here:
https://github.com/nats-io/nats-server/pull/4284/files#diff-e02d4430ca4d47be9db1b632ddb92d22c34c0919348bc020ce809e64fca8631cR2000

This is kinda hard to read and easy to mess up a struct with bunch of values of similar type.

Maybe add this linter?
https://golangci-lint.run/docs/linters/configuration/#exhaustruct

Signed-off-by: Alex Bozhenko <alexbozhenko@gmail.com>
@alexbozhenko alexbozhenko changed the title enalbe binary snapshots when server initally connects Enable binary snapshots on initial route connections Oct 28, 2025
neilalexander added a commit that referenced this pull request Oct 28, 2025
When routes to other servers are established `binarySnapshots` is
initially set to `false` indicating it is not supported. Then once a
`STATSZ` is received `binarySnapshots` will be updated to `true`.

Whether this server supports binary snapshots is determined by looping
over the peers and checking if other servers don't support it:
```
if sir, ok := s.nodeToInfo.Load(p.ID); ok && sir != nil && !sir.(nodeInfo).binarySnapshots
```

However, there's an issue here as the server will shortly think
`binarySnapshots` isn't supported and go back to the legacy snapshot
which contains interior deletes as a `[]uint64` which will be disastrous
if a stream has a huge amount of interior deletes. Since binary stream
snapshots are supported since 2.10.0, we can simply flip the
`binarySnapshots` default from `false` to `true`.

Alternative to #7476

Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
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