roachtest: backup/KMS/GCS/n3cpu4 failed-- SHOW EXPERIMENTAL FINGERPRINTS causes an OOM crash

[cockroach-teamcity]

roachtest.backup/KMS/GCS/n3cpu4 failed with artifacts on master @ 4d045594e8c65b56c82fcf2a1f14ee30cecfef3d:

(monitor.go:153).Wait: monitor failure: pq: query execution canceled
test artifacts and logs in: /artifacts/backup/KMS/GCS/n3cpu4/run_1

Parameters: ROACHTEST_arch=amd64 , ROACHTEST_cloud=gce , ROACHTEST_cpu=4 , ROACHTEST_encrypted=true , ROACHTEST_metamorphicBuild=true , ROACHTEST_ssd=0

Help

See: roachtest README

See: How To Investigate (internal)

See: Grafana

/cc @cockroachdb/disaster-recovery

This test on roachdash | Improve this report!

Jira issue: CRDB-33175

[msbutler]

According to 2.dmesg.txt, node 2 crashed with an OOM error. test.log indicates this OOM occurred after the backup/restore roundtrip and during a SHOW EXPERIMENTAL FINGERPRINTS query. It's worth noting:

• this test failed while running with metamorphic constants set to true. the test.log prints all the metamorphic constants that were set.
• A similar test failure occurred while run on AWS. I'm still trying to determine if an OOM caused this failure as well.

According to the latest heap profile in the artifacts, most memory allocation on node 2 looks to be in sql execution land:

I'm handing this over to sql queries to determine if this OOM points to a real bug in how they account for memory usage.

(cc @yuzefovich who has commented on a related slack thread)

[msbutler]

DR just had a another test failure with runtime assertions enabled where a SHOW EXPERIMENTAL FINGERPRINTS query ran out of the sql-memory budget (i.e. no OOM).

[yuzefovich]

I think there is something to look at. These OOMs seem to require a combination of very large coldata-batch-size metamorphic variable as well as another direct-scans-enabled being enabled (which is disabled by default). Upon a quick inspection, I'm not certain that we always do proper memory accounting in the local fast path of "direct columnar scans" (meaning that we might be not accounting for the right half of the heap profile). This issue is not super urgent, so for now I'll just disable metamorphic randomization of direct-scans-enabled.

[yuzefovich]

I think I see what happened here. In short, it's unlucky set of circumstances.

The OOM requires metamorphic direct-scans-enabled to be enabled. This cluster setting controls whether the "direct columnar scans" (aka KV projection pushdown) is enabled. With the bank workload with payload of 10KiB we hit the worst behavior of the direct columnar scans possible. Namely, we currently do not provide any hint to the CFetcherWrapper about the number of rows we're expected to scan (#94850); thus, it falls back to exponential growth of row count for the coldata.Batch starting at 1, and we allocate bathches of sizes 1, 2, ..., 512. However, because we set TargetBytes limit to 10MiB and we have roughly 10KiB per row, we expect to scan about 1k rows total, in a single BatchResponse. So we happen to allocate a fresh coldata.Batch every time and we never happen to reuse it. This is what the right side in the image above is about.

I did take a look at the memory accounting story, and I think we are covered in all cases (as commit message of b19a4a8 describes). In short,

• when we hit local fast path, that the footprint of not-serialized batches is included into BatchResponse.Size() and tracked by the txnKVFetcher
• when we serialize the batch, then on the KV server side we're tracking the memory footprint of a single batch (that we are reusing, modulo the batch growth as described above) against kv-mem monitor. On the SQL client side, it'll be again included into BatchResponse.Size().

Now, the left side of the heap profile is due to us casting payload column from STRING to BYTES. Because we're allocating a fresh coldata.Batch as described in the first paragraph, we also need to allocate a fresh vector for the BYTES type too. This vector is accounted for.

I think the reason for hitting the OOM here was that the roachtest uses --max-sql-memory=50% which is way too aggressive, so in some cases GOMEMLIMIT might not save us. I'll reduce that.

Another thing is that I don't think we actually need string::bytes cast in EXPERIMENTAL_FINGERPRINTS, so we should be able to reduce the footprint in the worst case by half. I'll still disable the direct columnar scans explicitly for this roachtest.