update CockroachDB to v21.2.9

[davepacheco]

This change updates Omicron from CockroachDB v21.1.10 to v21.2.9. Fixes #980, which should unblock #955. There are a few pieces:

• Changing the CockroachDB version and SHAs
• As @bnaecker mentioned on #980, CockroachDB now includes the default database name in the listen URL that it reports to us. This PR relaxes the code that parses this to be fine with that as long as the database name is "defaultdb". (The only reason this code is so paranoid in the first place is that I didn't want to accidentally throw away something important that was being provided in the listening URL.)
• I also changed the check in has_omicron_schema as mentioned here. From @mgartner's reply, I gather that we should always expect the undefined database error for this case now, and that does appear to be true.

Is there anything else we need to do before updating CockroachDB used by Omicron?

Then there's this last bit. In #980 @bnaecker mentioned:

Another possible oddity. I've noticed that the /tmp directory fills up extremely quickly. Like within two runs of the full test suite, whereas I'm not sure I'd ever hit an out-of-space error before. Something about the on-disk format may have changed that warrants further investigation.

This makes me a little nervous. I did some cursory digging, didn't find an issue, and I'm still thinking about how best to go ahead with this.

First of all, I think /tmp here is probably specific to @bnaecker's system (and probably the default behavior). I don't see any omicron-related files in there at all on my system. That's probably because I set TMPDIR=/dangerzone/omicron_tmp in my environment.

As far as I know, what goes in there during a clean cargo test is:

• whatever Cargo, rustc, ld, and other build tools put into temporary directories (which is unchanged by this change)
• log files from tests in progress (unchanged by this change)
• CockroachDB databases, copied from the seed in "target/debug" (could be changed by this change)

On success, I believe these should all be cleaned up -- there should normally be nothing left over. We've definitely had bugs where things forgot to clean up. On failures, this is all left around.

So I looked at the seed directory. After this change, mine is 2.71 MiB:

$ find target/debug -name crdb-base | xargs du -sh
2.71M	target/debug/build/nexus-test-utils-b8b82f169af47163/out/crdb-base

In other workspaces I have lying around (prior to this change), it's usually under 1 MiB. But that's small enough that I can't imagine it's filling up your /tmp.

Another possibility is that CockroachDB itself is using more memory (which comes from the same pool as /tmp, and so I believe could shrink the total size of /tmp) -- but again, that should get cleaned up after a successful run.

My inclination is to proceed with this change provided it passes CI, then separately check that we're cleaning up $TMPDIR properly after test runs. @bnaecker, I'm also happy to help dig into the /tmp problem when you have time. Let me know if you think it should block this, though.

[smklein]

Thanks for the PSA on the potential /tmp issue - we can keep an eye on it as we roll forward, and see if other devs / actions notice failures.

[bnaecker]

Yeah, I agree the tmp issue should not block this, since no one else has noticed anything out of order.

…

On 28 Apr 2022, at 12:27, Sean Klein ***@***.***> wrote:  @smklein approved this pull request. Thanks for the PSA on the potential /tmp issue - we can keep an eye on it as we roll forward, and see if other devs / actions notice failures. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.