server: sanitize the behavior of runtime stats sampling / dumps for secondary tenants #96168
Conversation
This was a simple mistake the first time the code was written. Release note: None
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
There was a problem hiding this comment.
There's already a test in tenant_vars_test.go that exercises the runtime stats on the tenant. If that test can be amended to use an in-process tenant, it should cover it.
Reviewed 1 of 1 files at r1, 4 of 4 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @knz)
Prior to this patch, the process-wide metrics were being sampled by each tenant server. This is unnecessary: they are process-wide so they can be sampled only once. Release note: None
Prior to this patch, in-flight trace dumps were disabled for secondary tenant servers. This patch re-enables them. The traces are written to a sub-directory of the configured job trace dump directory (`inflight_trace_dump` in the configured log directory). Release note: None
Great idea. Done. |
|
bors r=stevendanna,dhartunian |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
bors r=stevendanna,dhartunian |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build succeeded: |
Fixes #84589.
Epic: CRDB-14537
See the individual commits for details.