QiangCai opened a new pull request #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535 Be sure to do all of the following checklist to help us incorporate your contribution quickly and easily: - [ ] Any interfaces changed? - [ ] Any backward compatibility impacted? - [ ] Document update required? - [ ] Testing done Please provide details on - Whether new unit test cases have been added or why no new tests are required? - How it is tested? Please attach test report. - Is it a performance related change? Please attach the performance test report. - Any additional information to help reviewers in testing this change. - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
CarbonDataQA1 commented on issue #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#issuecomment-569257436 Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1307/ ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#issuecomment-569257438 Build Failed with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1298/ ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#issuecomment-569269032 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1318/ ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#issuecomment-569375557 Build Success with Spark 2.1.0, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.1/1306/ ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#issuecomment-569378599 Build Failed with Spark 2.2.1, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.2/1316/ ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
CarbonDataQA1 commented on issue #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#issuecomment-569379596 Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1329/ ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on issue #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#issuecomment-569512712 please retest this ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai edited a comment on issue #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#issuecomment-569512712 retest this please ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai removed a comment on issue #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#issuecomment-569512712 retest this please ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
QiangCai commented on issue #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#issuecomment-569513403 retest this please ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#discussion_r361857123 ########## File path: core/src/main/java/org/apache/carbondata/core/metadata/SegmentFileStore.java ########## @@ -1228,7 +1228,7 @@ public static void removeTempFolder(Map<String, FolderDetails> locationMap, Stri locationMap = new HashMap<>(); } - SegmentFile merge(SegmentFile mapper) { + public SegmentFile merge(SegmentFile mapper) { Review comment: name `mapper` is not good, can you rename it? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#discussion_r361857216 ########## File path: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ########## @@ -2730,39 +2734,66 @@ public static String encodeToString(byte[] bytes) throws UnsupportedEncodingExce return Base64.decodeBase64(objectString.getBytes(CarbonCommonConstants.DEFAULT_CHARSET)); } + public static void copyCarbonDataFileToCarbonStorePath(String localFilePath, + String carbonDataDirectoryPath, long fileSizeInBytes, Review comment: ```suggestion String targetPath, long fileSizeInBytes, ``` ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#discussion_r361857241 ########## File path: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ########## @@ -2730,39 +2734,66 @@ public static String encodeToString(byte[] bytes) throws UnsupportedEncodingExce return Base64.decodeBase64(objectString.getBytes(CarbonCommonConstants.DEFAULT_CHARSET)); } + public static void copyCarbonDataFileToCarbonStorePath(String localFilePath, + String carbonDataDirectoryPath, long fileSizeInBytes, + OutputFilesInfoHolder outputFilesInfoHolder) throws CarbonDataWriterException { + if (carbonDataDirectoryPath.endsWith(".tmp") && localFilePath Review comment: move localFilePath to next line ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#discussion_r361857272 ########## File path: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ########## @@ -2730,39 +2734,66 @@ public static String encodeToString(byte[] bytes) throws UnsupportedEncodingExce return Base64.decodeBase64(objectString.getBytes(CarbonCommonConstants.DEFAULT_CHARSET)); } + public static void copyCarbonDataFileToCarbonStorePath(String localFilePath, + String carbonDataDirectoryPath, long fileSizeInBytes, + OutputFilesInfoHolder outputFilesInfoHolder) throws CarbonDataWriterException { + if (carbonDataDirectoryPath.endsWith(".tmp") && localFilePath + .endsWith(CarbonCommonConstants.FACT_FILE_EXT)) { + // for partition case, write carbondata file directly to final path, keep index in temp path. + // This can improve the commit job performance on s3a. + carbonDataDirectoryPath = + carbonDataDirectoryPath.substring(0, carbonDataDirectoryPath.lastIndexOf("/")); + if (outputFilesInfoHolder != null) { + outputFilesInfoHolder.addToPartitionPath(carbonDataDirectoryPath); + } + } + long targetSize = copyCarbonDataFileToCarbonStorePath(localFilePath, carbonDataDirectoryPath, + fileSizeInBytes); + if (outputFilesInfoHolder != null) { + // Storing the number of files written by each task. + outputFilesInfoHolder.incrementCount(); + // Storing the files written by each task. + outputFilesInfoHolder.addToOutputFiles(carbonDataDirectoryPath + localFilePath + .substring(localFilePath.lastIndexOf(File.separator)) + ":" + targetSize); + } + } + /** * This method will copy the given file to carbon store location * * @param localFilePath local file name with full path * @throws CarbonDataWriterException Review comment: add @return description ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#discussion_r361857293 ########## File path: core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java ########## @@ -2730,39 +2734,66 @@ public static String encodeToString(byte[] bytes) throws UnsupportedEncodingExce return Base64.decodeBase64(objectString.getBytes(CarbonCommonConstants.DEFAULT_CHARSET)); } + public static void copyCarbonDataFileToCarbonStorePath(String localFilePath, + String carbonDataDirectoryPath, long fileSizeInBytes, + OutputFilesInfoHolder outputFilesInfoHolder) throws CarbonDataWriterException { + if (carbonDataDirectoryPath.endsWith(".tmp") && localFilePath + .endsWith(CarbonCommonConstants.FACT_FILE_EXT)) { + // for partition case, write carbondata file directly to final path, keep index in temp path. + // This can improve the commit job performance on s3a. + carbonDataDirectoryPath = + carbonDataDirectoryPath.substring(0, carbonDataDirectoryPath.lastIndexOf("/")); + if (outputFilesInfoHolder != null) { + outputFilesInfoHolder.addToPartitionPath(carbonDataDirectoryPath); + } + } + long targetSize = copyCarbonDataFileToCarbonStorePath(localFilePath, carbonDataDirectoryPath, + fileSizeInBytes); + if (outputFilesInfoHolder != null) { + // Storing the number of files written by each task. + outputFilesInfoHolder.incrementCount(); + // Storing the files written by each task. + outputFilesInfoHolder.addToOutputFiles(carbonDataDirectoryPath + localFilePath + .substring(localFilePath.lastIndexOf(File.separator)) + ":" + targetSize); + } + } + /** * This method will copy the given file to carbon store location * * @param localFilePath local file name with full path * @throws CarbonDataWriterException */ - public static void copyCarbonDataFileToCarbonStorePath(String localFilePath, + public static long copyCarbonDataFileToCarbonStorePath(String localFilePath, String carbonDataDirectoryPath, long fileSizeInBytes) throws CarbonDataWriterException { long copyStartTime = System.currentTimeMillis(); LOGGER.info(String.format("Copying %s to %s, operation id %d", localFilePath, carbonDataDirectoryPath, copyStartTime)); + long targetSize = 0; try { CarbonFile localCarbonFile = FileFactory.getCarbonFile(localFilePath); + long localFileSize = localCarbonFile.getSize(); // the size of local carbon file must be greater than 0 - if (localCarbonFile.getSize() == 0L) { + if (localFileSize == 0L) { LOGGER.error("The size of local carbon file: " + localFilePath + " is 0."); throw new CarbonDataWriterException("The size of local carbon file is 0."); } String carbonFilePath = carbonDataDirectoryPath + localFilePath .substring(localFilePath.lastIndexOf(File.separator)); copyLocalFileToCarbonStore(carbonFilePath, localFilePath, CarbonCommonConstants.BYTEBUFFER_SIZE, - getMaxOfBlockAndFileSize(fileSizeInBytes, localCarbonFile.getSize())); + getMaxOfBlockAndFileSize(fileSizeInBytes, localFileSize)); CarbonFile targetCarbonFile = FileFactory.getCarbonFile(carbonFilePath); // the size of carbon file must be greater than 0 // and the same as the size of local carbon file - if (targetCarbonFile.getSize() == 0L || - (targetCarbonFile.getSize() != localCarbonFile.getSize())) { + targetSize = targetCarbonFile.getSize(); + if (targetSize == 0L || + (targetSize != localFileSize)) { Review comment: ```suggestion targetSize != localFileSize) { ``` ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#discussion_r361857329 ########## File path: core/src/main/java/org/apache/carbondata/core/util/comparator/BigDecimalSerializableComparator.java ########## @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.carbondata.core.util.comparator; + +import java.math.BigDecimal; + +public class BigDecimalSerializableComparator implements SerializableComparator { + @Override + public int compare(Object key1, Object key2) { + if (key1 == null && key2 == null) { + return 0; + } else if (key1 == null) { + return -1; + } else if (key2 == null) { + return 1; + } + return ((BigDecimal) key1).compareTo((BigDecimal) key2); + } +} Review comment: add new line at the end ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#discussion_r361857386 ########## File path: core/src/main/java/org/apache/carbondata/core/writer/CarbonIndexFileMergeWriter.java ########## @@ -116,6 +119,66 @@ private String mergeCarbonIndexFilesOfSegment(String segmentId, return null; } + public SegmentFileStore.FolderDetails mergeCarbonIndexFilesOfSegment(String segmentId, Review comment: add description for the func ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#discussion_r361857417 ########## File path: hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonOutputCommitter.java ########## @@ -222,6 +215,91 @@ public void commitJob(JobContext context) throws IOException { } } + private void commitJobFinal(JobContext context, CarbonLoadModel loadModel, Review comment: add description ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
In reply to this post by GitBox
jackylk commented on a change in pull request #3535: [WIP] Refactory data loading for partition table
URL: https://github.com/apache/carbondata/pull/3535#discussion_r361857453 ########## File path: hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonOutputCommitter.java ########## @@ -222,6 +215,91 @@ public void commitJob(JobContext context) throws IOException { } } + private void commitJobFinal(JobContext context, CarbonLoadModel loadModel, + OperationContext operationContext, CarbonTable carbonTable, String uniqueId) + throws IOException { + DataMapStatusManager.disableAllLazyDataMaps(carbonTable); + if (operationContext != null) { + LoadEvents.LoadTablePostStatusUpdateEvent postStatusUpdateEvent = + new LoadEvents.LoadTablePostStatusUpdateEvent(loadModel); + try { + OperationListenerBus.getInstance() + .fireEvent(postStatusUpdateEvent, operationContext); + } catch (Exception e) { + throw new IOException(e); + } + } + String updateTime = + context.getConfiguration().get(CarbonTableOutputFormat.UPADTE_TIMESTAMP, null); + String segmentsToBeDeleted = + context.getConfiguration().get(CarbonTableOutputFormat.SEGMENTS_TO_BE_DELETED, ""); + List<Segment> segmentDeleteList = Segment.toSegmentList(segmentsToBeDeleted.split(","), null); + Set<Segment> segmentSet = new HashSet<>( + new SegmentStatusManager(carbonTable.getAbsoluteTableIdentifier(), + context.getConfiguration()).getValidAndInvalidSegments(carbonTable.isChildTableForMV()) + .getValidSegments()); + if (updateTime != null) { + CarbonUpdateUtil.updateTableMetadataStatus(segmentSet, carbonTable, updateTime, true, + segmentDeleteList); + } else if (uniqueId != null) { + CarbonUpdateUtil.updateTableMetadataStatus(segmentSet, carbonTable, uniqueId, true, + segmentDeleteList); + } + } + + private void commitJobForPartition(JobContext context, boolean overwriteSet, Review comment: add description ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [hidden email] With regards, Apache Git Services |
Free forum by Nabble | Edit this page |