Skip to content
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

release-2.1: gossip: avoid allocation of UnresolvedAddr in getNodeIDAddressLocked #30055

Merged

Conversation

nvanbenschoten
Copy link
Member

Backport 1/1 commits from #29585.

/cc @cockroachdb/release


getNodeIDAddressLocked is called from Dialer.ConnHealth and
Dialer.DialInternalClient. It was responsible for 1.71% of all
allocations (alloc_objects) on a 3-node long-running cluster that
was running TPC-C 1K.

Pointing into nd.LocalityAddress is safe because even if the NodeDescriptor
itself is replaced in Gossip, the struct is never internally mutated. This is
the same reason why taking the address of nd.Address was already safe.

Release note (performance improvement): Avoid allocation when
checking RPC connection health.

`getNodeIDAddressLocked` is called from `Dialer.ConnHealth` and
`Dialer.DialInternalClient`. It was responsible for 1.71% of all
allocations on a 3-node long running cluster that was running TPC-C 1K.

Pointing into `nd.LocalityAddress` is safe because even if the `NodeDescriptor`
itself is replaced in `Gossip`, the struct is never internally mutated. This is
the same reason why taking the address of `nd.Address` was already safe.

Release note (performance improvement): Avoid allocation when
checking RPC connection health.
@nvanbenschoten nvanbenschoten requested review from a-robinson and a team September 11, 2018 05:24
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@nvanbenschoten nvanbenschoten merged commit 517abb3 into cockroachdb:release-2.1 Sep 11, 2018
@nvanbenschoten nvanbenschoten deleted the backport2.1-29585 branch September 11, 2018 14:18
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.

3 participants