HIVE-28626: Display MoveTask/StatsTask duration on the query summary #5544
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
no, for me, an empty starting line is not expected in case of any methods, that's why I simply removed :)
| @@ -569,7 +569,8 @@ public int execute() { | |||
| } | |||
| releaseLocks(tbd); | |||
| } | |||
|
|
|||
| long moveFilesDuration = perfLogger.getDuration(PerfLogger.FILE_MOVES); | |||
There was a problem hiding this comment.
Two questions about this:
1.: This method already contains a perfLogEnd call:
// if _blob_files_kept is present, use it to move the files. Else fall back to normal case.
if (moveFilesUsingManifestFile(fs, sourcePath, targetPath)) {
perfLogger.perfLogEnd("MoveTask", PerfLogger.FILE_MOVES);
return;
}Does it stops the perfLogger? If yes, how that change will behave in that case?
2: perfLogEnd is already called at line 196. Can it affect the output of this getDuration call?
There was a problem hiding this comment.
- it doesn't stop, just registers and endTime for the duration of FILE_MOVES
- yeah, depending on the outcome, the ending time should still be registered anyways
possible refactoring is to fully encapsulate the whole movetask behavior to have a single begin + end call
abstractdog
force-pushed
the
HIVE-28626
branch
from
fdda590 to
e83ab6b
Compare
kokila-19
approved these changes
Nov 21, 2024
ayushtkn
approved these changes
Nov 23, 2024
abstractdog
force-pushed
the
HIVE-28626
branch
from
e83ab6b to
6e17233
Compare
asf-ci-hive
added
tests pending
tests unstable
and removed
tests passed
tests pending
labels
Change-Id: Ib37772938db5fdecf6507cdd43717a89f1e6250a
abstractdog
force-pushed
the
HIVE-28626
branch
from
6e17233 to
692a5c3
Compare
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Display MoveTask's and StatsTask's duration on the console.
Why are the changes needed?
Better observability.
Does this PR introduce any user-facing change?
Yes, query output on the console slightly changes.
Is the change a dependency upgrade?
No.
How was this patch tested?
On cluster. Unfortunately, due to HIVE-28627, the console message is broken, works only intermittently, but in the logs, the very same messages appear.