Add database admin scan command #3165
Conversation
andrewjdawson2016
force-pushed
the
dbAdmin
branch
from
3ac4248 to
e325764
Compare
tools/cli/adminDBCommands.go
Outdated
| scanWorkerCount = 40 | ||
| numberOfShards = 16384 | ||
| delayBetweenExecutionPages = 50 * time.Millisecond |
There was a problem hiding this comment.
could we make the first three configurable via CLI?
The scanWorkerCount and delayBetweenExecutionPages controls the QPS. As we exposed this, it would be better to be configurable as the db cluster could be different.
numberOfShards is based on the application config. So the operator can pass this in via CLI
There was a problem hiding this comment.
Sounds good - will do.
tools/cli/adminDBCommands.go
Outdated
| if combined.ShardID < report.ShardID { | ||
| combined.ShardID = report.ShardID | ||
| } |
There was a problem hiding this comment.
I don't quite understand this
There was a problem hiding this comment.
Will address.
tools/cli/adminDBCommands.go
Outdated
| combinedShardReport := &ScanShardReport{} | ||
| for i := 0; i < numberOfShards; i++ { | ||
| report := <-shardReports | ||
| combineShardReport(report, combinedShardReport) |
There was a problem hiding this comment.
what exactly is combined report ? why is it emitted for each shard ?
There was a problem hiding this comment.
Seems like this is confusing to multiple people. I will rethink how progress is reported.
tools/cli/adminDBCommands.go
Outdated
| const ( | ||
| scanWorkerCount = 40 | ||
| numberOfShards = 16384 | ||
| delayBetweenExecutionPages = 50 * time.Millisecond |
There was a problem hiding this comment.
I would prefer using a ratelimit (that is configurable with sane default) instead of this.
There was a problem hiding this comment.
Will make this change. Will use a dynamic rate limiter which starts at a low rps and over the course of X seconds moves to target rps.
tools/cli/adminDBCommands.go
Outdated
|
|
||
| const ( | ||
| scanWorkerCount = 40 | ||
| numberOfShards = 16384 |
There was a problem hiding this comment.
this is not true for all clusters. Can be a default, but would prefer this to be configurable as well
There was a problem hiding this comment.
good point.
andrewjdawson2016
force-pushed
the
dbAdmin
branch
from
f2d6608 to
14105a2
Compare
andrewjdawson2016
force-pushed
the
dbAdmin
branch
from
91f24a1 to
1926854
Compare
andrewjdawson2016
force-pushed
the
dbAdmin
branch
from
aaf8b2f to
a9521a0
Compare
andrewjdawson2016
force-pushed
the
dbAdmin
branch
from
61904cf to
e4f6af2
Compare
Added admin db scan command. And ListConcreteExecutions API to persistence.
This script is useful to detect data inconsistency.
If the rates used are too high it can interfere with production database access.