HADOOP-18635 : Expose distcp counters to user via new DistCpConstants "CONF_LABEL_DI… #5402
Conversation
This file contains hidden or 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
…STCP_TOTAL_BYTES_COPIED". The constant indicate number of bytes copied by distcp operation. It is exposed via configuration parameter through which user can obtain the value.
atsaonerk
changed the title
|
💔 -1 overall
This message was automatically generated. |
… after distcp is run.
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
steveloughran
requested changes
Feb 17, 2023
Contributor
steveloughran
left a comment
steveloughran
left a comment
There was a problem hiding this comment.
it's a pretty inelegant way of passing results back, isn't it: updating the conf file passed in.
I would be a lot happier if an IOStatisticsSnapshot was built up and returned, but that is a much bigger bit of work. (I'd actually like all iostats to be collected from workers and handed back...)
if you are going this way and others are happy with the idea, all the various counters should be returned this way and the distcp docs updated to cover this.
| ToolRunner.run(conf, distcpTool, args); | ||
| final long bytesCopied = NumberUtils.toLong(distcpTool.getConf(). | ||
| get(CONF_LABEL_DISTCP_TOTAL_BYTES_COPIED), 0); | ||
| assertEquals("Bytes copied by distcp tool should match source file length", |
Contributor
There was a problem hiding this comment.
assert args are the wrong way round for the generated error messages.
Corrected order of assert args in TestDistCpSystem.testDistcpLargeFile test.
|
🎊 +1 overall
This message was automatically generated. |
Contributor
|
I really don't like how the results come back. I'm going to propose adding IOStatistics support to distcp so lined up for future work and to not modify the source config to suddenly become two way exchange of data
This is extra work and you have to learn a new api, but
|
Contributor
|
We're closing this stale PR because it has been open for 100 days with no activity. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
…STCP_TOTAL_BYTES_COPIED".
The constant indicate number of bytes copied by distcp operation. It is exposed via configuration parameter through which user can obtain the value.
Description of PR
How was this patch tested?
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?