Skip to content

settings: store unencoded ClusterVersion in version setting#114087

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
RaduBerinde:store-cv
Nov 8, 2023
Merged

settings: store unencoded ClusterVersion in version setting#114087
craig[bot] merged 1 commit intocockroachdb:masterfrom
RaduBerinde:store-cv

Conversation

@RaduBerinde
Copy link
Copy Markdown
Member

We currently store the ClusterVersion as a protobuf encoded []byte. This requires unmarshaling in hot paths like IsActive.

This change switches to storing the ClusterVersion directly.

Fixes: #113385
Release note: None

We currently store the `ClusterVersion` as a protobuf encoded
`[]byte`. This requires unmarshaling in hot paths like `IsActive`.

This change switches to storing the `ClusterVersion` directly.

Fixes: cockroachdb#113385
Release note: None
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Collaborator

@pav-kv pav-kv left a comment

Choose a reason for hiding this comment

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

Thanks. Curious how much this improves BenchmarkClusterVersionSettingIsActive.

@RaduBerinde
Copy link
Copy Markdown
Member Author

TFTR!

On my macbook:

name                              old time/op  new time/op  delta
ClusterVersionSettingIsActive-10  25.2ns ± 0%  11.0ns ± 1%  -56.10%  (p=0.000 n=7+8)

bors r+

Copy link
Copy Markdown
Collaborator

@andyyang890 andyyang890 left a comment

Choose a reason for hiding this comment

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

LGTM!

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 8, 2023

Build succeeded:

@craig craig bot merged commit d969454 into cockroachdb:master Nov 8, 2023
@RaduBerinde RaduBerinde deleted the store-cv branch November 7, 2025 15:11
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.

clusterversion: proto Unmarshal on each version check

4 participants