Skip to content

roachprod: don't try to make long dns names#42986

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
dt:dns-len
Dec 5, 2019
Merged

roachprod: don't try to make long dns names#42986
craig[bot] merged 1 commit intocockroachdb:masterfrom
dt:dns-len

Conversation

@dt
Copy link
Copy Markdown
Contributor

@dt dt commented Dec 5, 2019

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.

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.
@dt dt requested a review from danhhz December 5, 2019 15:42
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@danhhz
Copy link
Copy Markdown
Contributor

danhhz commented Dec 5, 2019

for my understanding, does this fix the situation we're in now (aka engineers can pull in this change and resume using roachprod normally) or just prevent it in the future?

@dt
Copy link
Copy Markdown
Contributor Author

dt commented Dec 5, 2019

once anyone updates their version of roachprod, they should be able to update DNS, with the existing long cluster names as-is. Anyone who doesn't update will see an error, but that shouldn't block them or mess with anyone else -- they will just not have DNS for their clusters.

@dt
Copy link
Copy Markdown
Contributor Author

dt commented Dec 5, 2019

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 5, 2019

Build failed (retrying...)

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 24fadb2 into cockroachdb:master Dec 5, 2019
@dt dt deleted the dns-len branch December 5, 2019 22:52
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