-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
KYLIN-4833 create hfile on hadoop hdfs and distcp to hbase hdfs #1494
Conversation
This is a good idea. @guangxuCheng , would you like to have a look ? |
|
||
appendMapReduceParameters(cmd); | ||
appendExecCmdParameters(cmd, BatchConstants.ARG_CUBE_NAME, seg.getRealization().getName()); | ||
// appendExecCmdParameters(cmd, BatchConstants.ARG_PARTITION, getRowkeyDistributionOutputPath(jobId) + "/part-r-00000_hfile"); |
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.
remove commented code if not needed , will help in code readability going forward.
@@ -91,7 +91,8 @@ private void dropHdfsPathOnCluster(List<String> oldHdfsPaths, FileSystem fileSys | |||
} | |||
// If hbase was deployed on another cluster, the job dir is empty and should be dropped, | |||
// because of rowkey_stats and hfile dirs are both dropped. | |||
if (fileSystem.listStatus(oldPath.getParent()).length == 0) { | |||
Path parentPath = oldPath.getParent(); |
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.
good to use final if variable is not mutated : final Path parentPath
Configuration configuration = new Configuration(); | ||
HBaseConnection.addHBaseClusterNNHAConfiguration(configuration); | ||
|
||
Path input = new Path(getOptionValue(OPTION_INPUT_PATH)); |
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.
good to change to final
String partitionOutputPath = getRealizationRootPath(jobId) + "/rowkey_stats/part-r-00000_hfile"; | ||
appendExecCmdParameters(cmd, BatchConstants.ARG_PARTITION, partitionOutputPath); | ||
}else { | ||
appendExecCmdParameters(cmd, BatchConstants.ARG_PARTITION, |
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.
Append paramters twice for BatchConstants.ARG_PARTITION
.
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.
new commit fixed this
Codecov Report
@@ Coverage Diff @@
## master #1494 +/- ##
============================================
- Coverage 25.51% 25.43% -0.09%
+ Complexity 6765 6764 -1
============================================
Files 1505 1508 +3
Lines 93618 93871 +253
Branches 13111 13144 +33
============================================
- Hits 23886 23873 -13
- Misses 67356 67617 +261
- Partials 2376 2381 +5
Continue to review full report at Codecov.
|
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.
Approved
…he#1494) * create hfile on hadoop hdfs and distcp to hbase hdfs * code style Co-authored-by: zhangrusong <zhangrusong@ke.com>
* create hfile on hadoop hdfs and distcp to hbase hdfs * code style Co-authored-by: zhangrusong <zhangrusong@ke.com>
Proposed changes
Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.
Types of changes
What types of changes does your code introduce to Kylin?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.document
branchFurther comments
If this is a relatively large or complex change, kick off the discussion at user@kylin or dev@kylin by explaining why you chose the solution you did and what alternatives you considered, etc...