workloadccl: inject table statistics during fixtures load #39106
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @danhhz and @nvanbenschoten)
pkg/ccl/workloadccl/fixture.go, line 546 at r1 (raw file):
if injectStats && tablesHaveStats(tables) { // Turn off automatic stats temporarily so we don't trigger stats creation // after the IMPORT. We will inject stats inside importFixtureTable.
nit: RESTORE
pkg/ccl/workloadccl/fixture.go, line 550 at r1 (raw file):
// just trigger a no-op if there are new stats available so we wouldn't // have to disable and re-enable automatic stats here. enableFn := disableAutoStats(ctx, sqlDB)
this disableAutoStats and the defer need to go before the RESTORE jobs get kicked off, otherwise you have a race here where the auto stats could start up after the restore finishes but before this line
There was a problem hiding this comment.
Reviewable status:
pkg/ccl/workloadccl/fixture.go, line 545 at r1 (raw file):
tables := fixture.Generator.Tables() if injectStats && tablesHaveStats(tables) { // Turn off automatic stats temporarily so we don't trigger stats creation
I don't think this comment is accurate here.
In the existing code, we disabled auto stats before the import and ran the enableFn after the injection. I think we should do the same here (by the time this code runs, we may have started an auto stats job).
See cockroachdb#39103. Release note: None
nvanbenschoten
force-pushed
the
nvanbenschoten/injectOnBackup
branch
from
05ca887 to
0045120
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/ccl/workloadccl/fixture.go, line 545 at r1 (raw file):
Previously, RaduBerinde wrote…
I don't think this comment is accurate here.
In the existing code, we disabled auto stats before the import and ran the
enableFnafter the injection. I think we should do the same here (by the time this code runs, we may have started an auto stats job).
Done.
pkg/ccl/workloadccl/fixture.go, line 546 at r1 (raw file):
Previously, danhhz (Daniel Harrison) wrote…
nit: RESTORE
Done.
pkg/ccl/workloadccl/fixture.go, line 550 at r1 (raw file):
Previously, danhhz (Daniel Harrison) wrote…
this
disableAutoStatsand the defer need to go before the RESTORE jobs get kicked off, otherwise you have a race here where the auto stats could start up after the restore finishes but before this line
Done.
There was a problem hiding this comment.
Reviewable status:
|
bors r+ |
39106: workloadccl: inject table statistics during fixtures load r=nvanbenschoten a=nvanbenschoten See #39103. Release note: None Co-authored-by: Nathan VanBenschoten <[email protected]>
Build succeeded |
See #39103.
Release note: None