mmaprototype: add tracing to testing for rebalanceStores #157133
Conversation
|
This change is |
tbg
changed the title
tbg
force-pushed
the
mma-rebalance-refactor-auto
branch
2 times, most recently
from
af0a3a3 to
b852857
Compare
tbg
requested review from
a team,
Abhinav1299,
aa-joshi,
arjunmahishi,
jasonlmfong,
sumeerbhola and
wenyihu6
and removed request for
a team
sumeerbhola
left a comment
sumeerbhola
left a comment
There was a problem hiding this comment.
@sumeerbhola reviewed 3 of 3 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @aa-joshi, @Abhinav1299, @arjunmahishi, @jasonlmfong, and @wenyihu6)
wenyihu6
left a comment
wenyihu6
left a comment
There was a problem hiding this comment.
mod the comment
Reviewable status:
pkg/kv/kvserver/allocator/mmaprototype/cluster_state_test.go line 524 at r2 (raw file):
rng := rand.New(rand.NewSource(0)) dsm := newDiversityScoringMemo() tr := tracing.NewTracer()
do we need to call tr.Close in the end like this
?
tbg
force-pushed
the
mma-rebalance-refactor-auto
branch
from
b852857 to
0c5fa80
Compare
tbg
left a comment
tbg
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/kv/kvserver/allocator/mmaprototype/cluster_state_test.go line 524 at r2 (raw file):
Previously, wenyihu6 (Wenyi Hu) wrote…
do we need to call tr.Close in the end like this
?
We should, thanks. Added.
I'd like to make the trace events emitted from deterministic multi-metric allocator tests part of the expected test outputs. I realized there wasn't a standardized deterministic and barebones output format, so I added one. Epic: none
close tracer, ./dev gen bazel, set tracer redactable (make sure we get good debug output in production)
tbg
force-pushed
the
mma-rebalance-refactor-auto
branch
from
0c5fa80 to
754bacd
Compare
wenyihu6
left a comment
wenyihu6
left a comment
There was a problem hiding this comment.
Reviewable status:
157132: tracing: add minimal format suitable for datadriven tests r=wenyihu6 a=tbg I'd like to make the trace events emitted from deterministic multi-metric allocator tests part of the expected test outputs. I realized there wasn't a standardized deterministic and barebones output format, so I added one. In the test I have in mind I'll probably condense the file:line prefix further, but it seemed unsavory to slap regexp transformations into the tracing packages, and unfortunately that is required to solve the problem if we want to avoid changing the `log` package. Related to #157133. Epic: none 157150: cloud/azure: add optional azure client caching r=dt a=dt This adds a signle-slot, process-wide in-memory cache of the most recently used azure SDK storage client allowing subsequent attempts to open clients with the same configuration to reuse the already open client. This can allow the SDK's internal caching of things like IMDS tokens to be used across separate CRDB operations on separate files which currently each open their own client. This can be particularly impactful for avoiding hitting the IMDS rate limit. Release note: none. Epic: none. 157217: mmaprototype: move rebalanceStores to its own file r=wenyihu6 a=tbg Epic: CRDB-55052 Co-authored-by: Tobias Grieger <[email protected]> Co-authored-by: David Taylor <[email protected]>
|
TFTR! bors r+ |
|
Ugh, wanted to unstack this before merging and squash the fixup commit. Making a mental note to add do-not-merge label when in that state. |
4 participants
This PR adds the basic capability to exercise rebalanceStores through
TestClusterState. It does this by moving rebalanceStores from allocatorState to
clusterState, which is a small and mostly mechanical refactor.
The immediate motivation behind this change is being able to exercise
the functionality for #156776 while it is being added, while being able
to see the internal tracepoints as well.
Informs #156776.
Epic: CRDB-55052