Skip to content

settings: explanatory comments around cluster settings#42991

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
irfansharif:191205.IsActive-comment
Dec 5, 2019
Merged

settings: explanatory comments around cluster settings#42991
craig[bot] merged 1 commit intocockroachdb:masterfrom
irfansharif:191205.IsActive-comment

Conversation

@irfansharif
Copy link
Copy Markdown
Contributor

Update stale references to changed method signatures and add a bit more
documentation around the usage of IsActive.

Release note: None

@irfansharif irfansharif requested a review from nvb December 5, 2019 16:58
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@nvb nvb left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @irfansharif)


pkg/settings/cluster/settings.go, line 99 at r1 (raw file):

//
// During the node startup sequence, an initial version (persisted to the
// engines) is read and passed to Initialize(). It is only after that

Mind rewrapping this?

Update stale references to changed method signatures and add a bit more
documentation around the usage of IsActive.

Release note: None
@irfansharif irfansharif force-pushed the 191205.IsActive-comment branch from b36fbf0 to e7a492e Compare December 5, 2019 20:52
Copy link
Copy Markdown
Contributor Author

@irfansharif irfansharif left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @nvanbenschoten)


pkg/settings/cluster/settings.go, line 99 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Mind rewrapping this?

On a scale of 1-10, how strongly do you feel about this? 😛

@irfansharif
Copy link
Copy Markdown
Contributor Author

bors r+

craig bot pushed a commit that referenced this pull request Dec 5, 2019
42985: issues: simplify post() r=jordanlewis a=tbg

This prepares the code for allowing the caller more control over the
message body. This makes sense since post() is called both by roachtest
and nightly stress, and it doesn't make sense to use the same message
body for both.

Release note: None

42986: roachprod: don't try to make long dns names r=dt a=dt

long cluster names break DNS. Rather than break DNS for everyone, just skip the clusters that have these long names.
DNS is a nice-to-have in any case, so it is OK to skip it -- roachprod will fallback to IPs.

Release note: none.

42991: settings: explanatory comments around cluster settings r=irfansharif a=irfansharif

Update stale references to changed method signatures and add a bit more
documentation around the usage of IsActive.

Release note: None

42998: sql: bugfixes in role membership caching r=petermattis a=rohany

Fixes #42098.

This PR fixes some bugs in role membership caching.

* The role membership cache was a single global, which meant
  multiple servers in the same cockroach process could pollute
  the cache for each other, causing flaky test failures.
* The cache updating mechanism seemed to use the incorrect variable
  to update the cache version when a concurrent request caused a cache
  invalidation as well. Although unlikely, this could have lead to incorrect
  results being served.

Release note (bug fix): This change fixes some existing caching issues
surrounding role memberships, where users could sometimes see out of date
role membership information.

Co-authored-by: Tobias Schottdorf <tobias.schottdorf@gmail.com>
Co-authored-by: David Taylor <tinystatemachine@gmail.com>
Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 5, 2019

Build succeeded

@craig craig bot merged commit e7a492e into cockroachdb:master Dec 5, 2019
@irfansharif irfansharif deleted the 191205.IsActive-comment branch December 9, 2019 02:13
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