-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Optimise metalayer snapshots #4925
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. I left a comment on whether we should remove client info from snapshots for streams and consumers. We don't need it at that point and probably should not store it.
How are performance results looking?
Config: sa.Config, | ||
Group: sa.Group, | ||
Sync: sa.Sync, | ||
Client: sa.Client, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we need client here for snapshots? Both for streams and consumers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do, because there are quite a few places that stream assignments check for the service account and other fields once rehydrated from the snapshot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct we need account, but not much else I believe.
Signed-off-by: Neil Twigg <neil@nats.io>
Signed-off-by: Neil Twigg <neil@nats.io>
50da2df
to
beb0e1a
Compare
Let me know if you want a review now for merge. |
Think this is fine for review to merge for now, looking into the |
Signed-off-by: Neil Twigg <neil@nats.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR optimises the marshalling of metalayer snapshots with a few techniques:
metaSnapshot()
by preallocating slicesSigned-off-by: Neil Twigg neil@nats.io