-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restore: add and fill host info in restore progress #4088
Conversation
Examples: Before first load&stream
During restore
During repair
|
Why the kilobytes is (160.801k/s/shard) instead of (160.801kB/s/shard) (813/s/shard) is bytes per second per shard ? B is missing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Michal-Leszczynski idle time per node is missing
I just used the standard way in which we display bytes in managerclient.
That's true! I forgot about your suggestion about that in the issue. Unfortunately, it can't be calculated like Of course, we could manually calculate entire task execution duration by traversing previous runs (on SM side), but even that won't solve the whole problem. Another thing with this approach is that the total task execution duration also includes other time consuming restore stages (e.g. indexing, changing In order to overcome that, we would need to know when download and load&stream stage started and finished, but we currently don't have such information. For those reasons, I would prefer to skip |
97fd869
to
f960de8
Compare
Having information that node was spending time on something other that l&s or download helps with finding potential optimizations for the restore. Examples from 3.3.3 shows that node can download fast, l&s fast but remain idle for most of the time.
Yes, then let's do it.
Call it |
@Michal-Leszczynski as per today's sync. It's enough to include the time node spent on l&s and the time node spent on downloading. |
f960de8
to
731eb38
Compare
@karol-kokoszka here is updated display:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
731eb38
to
cfce92a
Compare
@karol-kokoszka I just discovered a small display bug.
I just fixed that. |
It was a left-over after feature development:/
It's going to be needed for calculating per shard download/stream bandwidth in progress command.
This commit also moves host shard info to the tablesWorker, as it is commonly reused during restore procedure.
This allows to calculate download/stream per shard bandwidth in 'sctool progress' display.
It is nicer to see: "Size: 10B" instead of "Size: 10" or "Size: 20KiB" instead of "Size: 20k".
cfce92a
to
44a2b80
Compare
Restore: add and fill host info in restore progress * chore(go.mod): remove replace directive to SM submodules It was a left-over after feature development:/ * chore(go.mod): bump SM submodules deps * feat(schema): add shard cnt to restore_run_progress It's going to be needed for calculating per shard download/stream bandwidth in progress command. * feat(restore): add and fill shard cnt in restore run progress This commit also moves host shard info to the tablesWorker, as it is commonly reused during restore procedure. * feat(restore): add and fill host info in progress This allows to calculate download/stream per shard bandwidth in 'sctool progress' display. * feat(managerclient): display bandwidth in sctool progress Fixes #4042 * feat(managerclient): include B or iB in SizeSuffix display It is nicer to see: "Size: 10B" instead of "Size: 10" or "Size: 20KiB" instead of "Size: 20k".
This PR creates and fills new swagger host restore progress definitions from #4082 in restore service.
It also updates managerclient to correctly display bandwidth and shard information in
sctool progress
.Fixes #4042