You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by GitBox <gi...@apache.org> on 2020/02/06 09:30:39 UTC
[GitHub] [carbondata] jackylk opened a new pull request #3606: [WIP] add
compressor to file name and change default compressor to zstd
jackylk opened a new pull request #3606: [WIP] add compressor to file name and change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606
### Why is this PR needed?
WIP
### What changes were proposed in this PR?
### Does this PR introduce any user interface change?
- No
- Yes. (please explain the change and update document)
### Is any new testcase added?
- No
- Yes
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583357088
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1880/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r377100880
##########
File path: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java
##########
@@ -285,17 +286,38 @@ public static String getSegmentPath(String tablePath, String segmentId) {
}
/**
- * Gets data file name only with out path
- *
- * @param filePartNo data file part number
- * @param taskNo task identifier
- * @param factUpdateTimeStamp unique identifier to identify an update
- * @return gets data file name only with out path
+ * Gets data file name only, without parent path
*/
public static String getCarbonDataFileName(Integer filePartNo, String taskNo, int bucketNumber,
- int batchNo, String factUpdateTimeStamp, String segmentNo) {
- return DATA_PART_PREFIX + filePartNo + "-" + taskNo + BATCH_PREFIX + batchNo + "-"
- + bucketNumber + "-" + segmentNo + "-" + factUpdateTimeStamp + CARBON_DATA_EXT;
+ int batchNo, String factUpdateTimeStamp, String segmentNo, String compressor) {
+ Objects.requireNonNull(filePartNo);
+ Objects.requireNonNull(taskNo);
+ Objects.requireNonNull(factUpdateTimeStamp);
+ Objects.requireNonNull(compressor);
+
+ // Start from CarbonData 2.0, the data file name patten is:
+ // partNo-taskNo-batchNo-bucketNo-segmentNo-timestamp.compressor.carbondata
+ // For example:
+ // part-0-0_batchno0-0-0-1580982686749.zstd.carbondata
+ //
+ // If the compressor name is missing, the file is compressed by snappy, which is
+ // the default compressor in CarbonData 1.x
+
+ return new StringBuilder()
+ .append(DATA_PART_PREFIX)
+ .append(filePartNo)
+ .append("-")
+ .append(taskNo)
+ .append(BATCH_PREFIX)
+ .append(batchNo)
+ .append("-")
+ .append(bucketNumber)
+ .append("-")
+ .append(segmentNo)
+ .append("-")
+ .append(factUpdateTimeStamp)
Review comment:
yes, I wanted to test once without changing the file name, in order to simulate using new jar to test on old files.
As the CI is passed, so that the compatibility is ok.
I will add the compressor name in file name now
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606: [WIP] add
compressor to file name and change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [WIP] add compressor to file name and change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583002412
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/172/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#discussion_r383084187
##########
File path: streaming/src/main/java/org/apache/carbondata/streaming/CarbonStreamRecordWriter.java
##########
@@ -139,7 +139,9 @@ private void initialize(TaskAttemptContext job) throws IOException {
segmentDir = CarbonTablePath.getSegmentPath(
carbonTable.getAbsoluteTableIdentifier().getTablePath(), segmentId);
- fileName = CarbonTablePath.getCarbonDataFileName(0, taskNo + "", 0, 0, "0", segmentId);
+ fileName = CarbonTablePath.getCarbonDataFileName(
+ 0, taskNo + "", 0, 0, "0",
+ segmentId, CompressorFactory.NativeSupportedCompressor.SNAPPY.getName());
Review comment:
fixed
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606: [WIP] add
compressor to file name and change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [WIP] add compressor to file name and change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-582910031
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/171/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583758880
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/184/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606: [WIP] add
compressor to file name and change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [WIP] add compressor to file name and change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-582822047
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/165/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name
and change default compressor to ZSTD
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name and change default compressor to ZSTD
URL: https://github.com/apache/carbondata/pull/3606#discussion_r382908617
##########
File path: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java
##########
@@ -409,11 +433,10 @@ public static String getTimeStampFromFileName(String carbonDataFileName) {
* @return
*/
public static Boolean compareCarbonFileTimeStamp(String fileName, Long timestamp) {
- int lastIndexOfHyphen = fileName.lastIndexOf("-");
- int lastIndexOfDot = fileName.lastIndexOf(".");
- if (lastIndexOfHyphen > 0 && lastIndexOfDot > 0) {
- return fileName.substring(fileName.lastIndexOf("-") + 1, fileName.lastIndexOf("."))
- .equals(timestamp.toString());
+ int startIndex = fileName.lastIndexOf("-");
Review comment:
fixed
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name
and change default compressor to ZSTD
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name and change default compressor to ZSTD
URL: https://github.com/apache/carbondata/pull/3606#discussion_r382908617
##########
File path: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java
##########
@@ -409,11 +433,10 @@ public static String getTimeStampFromFileName(String carbonDataFileName) {
* @return
*/
public static Boolean compareCarbonFileTimeStamp(String fileName, Long timestamp) {
- int lastIndexOfHyphen = fileName.lastIndexOf("-");
- int lastIndexOfDot = fileName.lastIndexOf(".");
- if (lastIndexOfHyphen > 0 && lastIndexOfDot > 0) {
- return fileName.substring(fileName.lastIndexOf("-") + 1, fileName.lastIndexOf("."))
- .equals(timestamp.toString());
+ int startIndex = fileName.lastIndexOf("-");
Review comment:
This func is not used anywhere, I deleted 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on a change in pull request #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
niuge01 commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r376770522
##########
File path: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java
##########
@@ -285,17 +286,39 @@ public static String getSegmentPath(String tablePath, String segmentId) {
}
/**
- * Gets data file name only with out path
- *
- * @param filePartNo data file part number
- * @param taskNo task identifier
- * @param factUpdateTimeStamp unique identifier to identify an update
- * @return gets data file name only with out path
+ * Gets data file name only, without parent path
*/
public static String getCarbonDataFileName(Integer filePartNo, String taskNo, int bucketNumber,
- int batchNo, String factUpdateTimeStamp, String segmentNo) {
- return DATA_PART_PREFIX + filePartNo + "-" + taskNo + BATCH_PREFIX + batchNo + "-"
- + bucketNumber + "-" + segmentNo + "-" + factUpdateTimeStamp + CARBON_DATA_EXT;
+ int batchNo, String factUpdateTimeStamp, String segmentNo, String compressor) {
+ Objects.requireNonNull(filePartNo);
+ Objects.requireNonNull(taskNo);
+ Objects.requireNonNull(factUpdateTimeStamp);
+ Objects.requireNonNull(compressor);
+
+ // Start from CarbonData 2.0, the data file name patten is:
+ // partNo-taskNo-batchNo-bucketNo-segmentNo-timestamp.compressor.carbondata
+ // For example:
+ // part-0-0_batchno0-0-0-1580982686749.zstd.carbondata
+ //
+ // If the compressor name is missing, the file is compressed by snappy, which is
+ // the default compressor in CarbonData 1.x
+
+ return new StringBuffer().append(DATA_PART_PREFIX)
Review comment:
There is need not use StringBuffer to build string, just use string concat will be ok.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-589954095
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/399/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r377103325
##########
File path: streaming/src/main/java/org/apache/carbondata/streaming/CarbonStreamRecordWriter.java
##########
@@ -140,7 +140,9 @@ private void initialize(TaskAttemptContext job) throws IOException {
segmentDir = CarbonTablePath.getSegmentPath(
carbonTable.getAbsoluteTableIdentifier().getTablePath(), segmentId);
- fileName = CarbonTablePath.getCarbonDataFileName(0, taskNo + "", 0, 0, "0", segmentId);
+ fileName = CarbonTablePath.getCarbonDataFileName(
Review comment:
There is testcase failing if set to ZSTD. Need another PR for 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] QiangCai commented on issue #3606: [CARBONDATA-3681]
Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
QiangCai commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583934608
better to do further testing on a cluster, not only a local machine.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583860628
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/200/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request
#3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file
name and change default compressor to ZSTD
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name and change default compressor to ZSTD
URL: https://github.com/apache/carbondata/pull/3606#discussion_r382532147
##########
File path: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java
##########
@@ -409,11 +433,10 @@ public static String getTimeStampFromFileName(String carbonDataFileName) {
* @return
*/
public static Boolean compareCarbonFileTimeStamp(String fileName, Long timestamp) {
- int lastIndexOfHyphen = fileName.lastIndexOf("-");
- int lastIndexOfDot = fileName.lastIndexOf(".");
- if (lastIndexOfHyphen > 0 && lastIndexOfDot > 0) {
- return fileName.substring(fileName.lastIndexOf("-") + 1, fileName.lastIndexOf("."))
- .equals(timestamp.toString());
+ int startIndex = fileName.lastIndexOf("-");
Review comment:
Suggest to use CarbonCommonConstants for "-". "."
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-589960864
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2100/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on a change in pull request #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
niuge01 commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r376770113
##########
File path: core/src/main/java/org/apache/carbondata/core/readcommitter/LatestFilesReadCommittedScope.java
##########
@@ -163,7 +163,7 @@ public SegmentRefreshInfo getCommittedSegmentRefreshInfo(Segment segment, Update
return segmentRefreshInfo;
}
- private String getSegmentID(String carbonIndexFileName, String indexFilePath) {
+ private String getTimestamp(String carbonIndexFileName, String indexFilePath) {
Review comment:
Why change method name to getTimestamp?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583838300
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/193/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name
and change default compressor to ZSTD
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name and change default compressor to ZSTD
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-584197385
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/219/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583844201
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1895/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name
and change default compressor to ZSTD
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name and change default compressor to ZSTD
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-589554209
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/386/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r377103094
##########
File path: streaming/src/main/java/org/apache/carbondata/streaming/CarbonStreamRecordWriter.java
##########
@@ -188,7 +190,7 @@ private void initializeAtFirstRow() throws IOException {
compressorName = carbonTable.getTableInfo().getFactTable().getTableProperties().get(
CarbonCommonConstants.COMPRESSOR);
if (null == compressorName) {
- compressorName = CompressorFactory.getInstance().getCompressor().getName();
+ compressorName = CompressorFactory.NativeSupportedCompressor.SNAPPY.getName();
Review comment:
There is testcase failing if set to ZSTD. Need another PR for 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606: [WIP] add
compressor to file name and change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [WIP] add compressor to file name and change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583027918
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1874/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r376776975
##########
File path: core/src/main/java/org/apache/carbondata/core/readcommitter/LatestFilesReadCommittedScope.java
##########
@@ -163,7 +163,7 @@ public SegmentRefreshInfo getCommittedSegmentRefreshInfo(Segment segment, Update
return segmentRefreshInfo;
}
- private String getSegmentID(String carbonIndexFileName, String indexFilePath) {
+ private String getTimestamp(String carbonIndexFileName, String indexFilePath) {
Review comment:
I changed back
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] asfgit closed pull request #3606: [CARBONDATA-3688]
Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on issue #3606:
[CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on issue #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-590209666
LGTM
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r377102667
##########
File path: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/table/CarbonDescribeFormattedCommand.scala
##########
@@ -141,11 +141,6 @@ private[sql] case class CarbonDescribeFormattedCommand(
Strings.formatSize(
tblProps.getOrElse(CarbonCommonConstants.CARBON_LOAD_MIN_SIZE_INMB,
CarbonCommonConstants.CARBON_LOAD_MIN_SIZE_INMB_DEFAULT).toFloat), ""),
- ("Data File Compressor ", tblProps
Review comment:
Because the default compressor changed, so this print is not correct now. And there is no way to know the compressor unless you check the file name. I think this is better since very loading can use different compressor, so we should not show in DESC which is table level
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3606: [CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r376990515
##########
File path: streaming/src/main/java/org/apache/carbondata/streaming/CarbonStreamRecordWriter.java
##########
@@ -188,7 +190,7 @@ private void initializeAtFirstRow() throws IOException {
compressorName = carbonTable.getTableInfo().getFactTable().getTableProperties().get(
CarbonCommonConstants.COMPRESSOR);
if (null == compressorName) {
- compressorName = CompressorFactory.getInstance().getCompressor().getName();
+ compressorName = CompressorFactory.NativeSupportedCompressor.SNAPPY.getName();
Review comment:
why always snappy for streaming segment ? before I think it was supporting configured compressor
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3606: [CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#discussion_r382961481
##########
File path: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/describeTable/TestDescribeTable.scala
##########
@@ -68,15 +68,6 @@ class TestDescribeTable extends QueryTest with BeforeAndAfterAll {
assert(descPar.exists(_.toString().contains("Partition Parameters:")))
}
- test(testName = "Compressor Type update from carbon properties") {
- sql("drop table if exists b")
- sql(sqlText = "create table b(a int,b string) STORED AS carbondata")
- CarbonProperties.getInstance().addProperty(CarbonCommonConstants.COMPRESSOR, "gzip")
- val result = sql(sqlText = "desc formatted b").collect()
- assert(result.filter(row => row.getString(0).contains("Data File Compressor")).head.getString
Review comment:
As now we are not changing the default compressor, we can revert this change ?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name
and change default compressor to ZSTD
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name and change default compressor to ZSTD
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-584234139
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1921/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] niuge01 commented on a change in pull request #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
niuge01 commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r376769368
##########
File path: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##########
@@ -1083,7 +1083,7 @@ private CarbonCommonConstants() {
* The optional values are 'SNAPPY','GZIP','BZIP2','LZ4','ZSTD' and empty.
* Specially, empty means that Carbondata will not compress the sort temp files.
*/
- public static final String CARBON_SORT_TEMP_COMPRESSOR_DEFAULT = "SNAPPY";
+ public static final String CARBON_SORT_TEMP_COMPRESSOR_DEFAULT = "zstd";
Review comment:
```suggestion
public static final String CARBON_SORT_TEMP_COMPRESSOR_DEFAULT = "ZSTD";
```
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583289470
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/176/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r377101401
##########
File path: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/datacompaction/TableLevelCompactionOptionTest.scala
##########
@@ -139,6 +139,8 @@ class TableLevelCompactionOptionTest extends QueryTest
sql(s"LOAD DATA LOCAL INPATH '$tempFilePath' INTO TABLE carbon_table")
}
+ sql("SHOW SEGMENTS FOR TABLE carbon_table").show
Review comment:
fixed
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583925032
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/203/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name
and change default compressor to ZSTD
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name and change default compressor to ZSTD
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-589588444
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2087/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3606: [CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r376988998
##########
File path: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/datacompaction/TableLevelCompactionOptionTest.scala
##########
@@ -139,6 +139,8 @@ class TableLevelCompactionOptionTest extends QueryTest
sql(s"LOAD DATA LOCAL INPATH '$tempFilePath' INTO TABLE carbon_table")
}
+ sql("SHOW SEGMENTS FOR TABLE carbon_table").show
Review comment:
revert this or add validation
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#discussion_r383084181
##########
File path: streaming/src/main/java/org/apache/carbondata/streaming/CarbonStreamRecordWriter.java
##########
@@ -186,7 +188,7 @@ private void initializeAtFirstRow() throws IOException {
compressorName = carbonTable.getTableInfo().getFactTable().getTableProperties().get(
CarbonCommonConstants.COMPRESSOR);
if (null == compressorName) {
- compressorName = CompressorFactory.getInstance().getCompressor().getName();
+ compressorName = CompressorFactory.NativeSupportedCompressor.SNAPPY.getName();
Review comment:
fixed
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name
and change default compressor to ZSTD
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name and change default compressor to ZSTD
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-585133694
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/257/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3606: [CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#discussion_r382961552
##########
File path: streaming/src/main/java/org/apache/carbondata/streaming/CarbonStreamRecordWriter.java
##########
@@ -139,7 +139,9 @@ private void initialize(TaskAttemptContext job) throws IOException {
segmentDir = CarbonTablePath.getSegmentPath(
carbonTable.getAbsoluteTableIdentifier().getTablePath(), segmentId);
- fileName = CarbonTablePath.getCarbonDataFileName(0, taskNo + "", 0, 0, "0", segmentId);
+ fileName = CarbonTablePath.getCarbonDataFileName(
+ 0, taskNo + "", 0, 0, "0",
+ segmentId, CompressorFactory.NativeSupportedCompressor.SNAPPY.getName());
Review comment:
revert this or use CCC default compressor
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-590169024
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/419/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file
name and change default compressor to ZSTD
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name and change default compressor to ZSTD
URL: https://github.com/apache/carbondata/pull/3606#discussion_r377419646
##########
File path: streaming/src/main/java/org/apache/carbondata/streaming/CarbonStreamRecordWriter.java
##########
@@ -188,7 +190,7 @@ private void initializeAtFirstRow() throws IOException {
compressorName = carbonTable.getTableInfo().getFactTable().getTableProperties().get(
CarbonCommonConstants.COMPRESSOR);
if (null == compressorName) {
- compressorName = CompressorFactory.getInstance().getCompressor().getName();
+ compressorName = CompressorFactory.NativeSupportedCompressor.SNAPPY.getName();
Review comment:
ok, please add a TODO
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#discussion_r383083186
##########
File path: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/describeTable/TestDescribeTable.scala
##########
@@ -68,15 +68,6 @@ class TestDescribeTable extends QueryTest with BeforeAndAfterAll {
assert(descPar.exists(_.toString().contains("Partition Parameters:")))
}
- test(testName = "Compressor Type update from carbon properties") {
- sql("drop table if exists b")
- sql(sqlText = "create table b(a int,b string) STORED AS carbondata")
- CarbonProperties.getInstance().addProperty(CarbonCommonConstants.COMPRESSOR, "gzip")
- val result = sql(sqlText = "desc formatted b").collect()
- assert(result.filter(row => row.getString(0).contains("Data File Compressor")).head.getString
Review comment:
No, this print is not correct. For example, when loading, user set to use ZSTD compressor, and then set to SNAPPY again after loading, then this command still prints SNAPPY which is misleading. Compressor should be file level by checking the file name or the meta inside the file
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r376777589
##########
File path: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java
##########
@@ -285,17 +286,39 @@ public static String getSegmentPath(String tablePath, String segmentId) {
}
/**
- * Gets data file name only with out path
- *
- * @param filePartNo data file part number
- * @param taskNo task identifier
- * @param factUpdateTimeStamp unique identifier to identify an update
- * @return gets data file name only with out path
+ * Gets data file name only, without parent path
*/
public static String getCarbonDataFileName(Integer filePartNo, String taskNo, int bucketNumber,
- int batchNo, String factUpdateTimeStamp, String segmentNo) {
- return DATA_PART_PREFIX + filePartNo + "-" + taskNo + BATCH_PREFIX + batchNo + "-"
- + bucketNumber + "-" + segmentNo + "-" + factUpdateTimeStamp + CARBON_DATA_EXT;
+ int batchNo, String factUpdateTimeStamp, String segmentNo, String compressor) {
+ Objects.requireNonNull(filePartNo);
+ Objects.requireNonNull(taskNo);
+ Objects.requireNonNull(factUpdateTimeStamp);
+ Objects.requireNonNull(compressor);
+
+ // Start from CarbonData 2.0, the data file name patten is:
+ // partNo-taskNo-batchNo-bucketNo-segmentNo-timestamp.compressor.carbondata
+ // For example:
+ // part-0-0_batchno0-0-0-1580982686749.zstd.carbondata
+ //
+ // If the compressor name is missing, the file is compressed by snappy, which is
+ // the default compressor in CarbonData 1.x
+
+ return new StringBuffer().append(DATA_PART_PREFIX)
Review comment:
I changed to StringBuilder, and this link (https://stackoverflow.com/questions/47605/string-concatenation-concat-vs-operator) suggest StringBuilder is more efficient
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583935130
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1905/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#discussion_r383083275
##########
File path: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/table/CarbonDescribeFormattedCommand.scala
##########
@@ -141,11 +141,6 @@ private[sql] case class CarbonDescribeFormattedCommand(
Strings.formatSize(
tblProps.getOrElse(CarbonCommonConstants.CARBON_LOAD_MIN_SIZE_INMB,
CarbonCommonConstants.CARBON_LOAD_MIN_SIZE_INMB_DEFAULT).toFloat), ""),
- ("Data File Compressor ", tblProps
- .getOrElse(CarbonCommonConstants.COMPRESSOR,
- CarbonProperties.getInstance()
Review comment:
explained above
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583337934
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/177/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on issue #3606: [CARBONDATA-3681]
Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
jackylk commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583959274
@QiangCai ok, I will verify on cluster
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606: [WIP] add
compressor to file name and change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [WIP] add compressor to file name and change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-582933159
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1873/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3606: [CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r376948840
##########
File path: core/src/main/java/org/apache/carbondata/core/util/path/CarbonTablePath.java
##########
@@ -285,17 +286,38 @@ public static String getSegmentPath(String tablePath, String segmentId) {
}
/**
- * Gets data file name only with out path
- *
- * @param filePartNo data file part number
- * @param taskNo task identifier
- * @param factUpdateTimeStamp unique identifier to identify an update
- * @return gets data file name only with out path
+ * Gets data file name only, without parent path
*/
public static String getCarbonDataFileName(Integer filePartNo, String taskNo, int bucketNumber,
- int batchNo, String factUpdateTimeStamp, String segmentNo) {
- return DATA_PART_PREFIX + filePartNo + "-" + taskNo + BATCH_PREFIX + batchNo + "-"
- + bucketNumber + "-" + segmentNo + "-" + factUpdateTimeStamp + CARBON_DATA_EXT;
+ int batchNo, String factUpdateTimeStamp, String segmentNo, String compressor) {
+ Objects.requireNonNull(filePartNo);
+ Objects.requireNonNull(taskNo);
+ Objects.requireNonNull(factUpdateTimeStamp);
+ Objects.requireNonNull(compressor);
+
+ // Start from CarbonData 2.0, the data file name patten is:
+ // partNo-taskNo-batchNo-bucketNo-segmentNo-timestamp.compressor.carbondata
+ // For example:
+ // part-0-0_batchno0-0-0-1580982686749.zstd.carbondata
+ //
+ // If the compressor name is missing, the file is compressed by snappy, which is
+ // the default compressor in CarbonData 1.x
+
+ return new StringBuilder()
+ .append(DATA_PART_PREFIX)
+ .append(filePartNo)
+ .append("-")
+ .append(taskNo)
+ .append(BATCH_PREFIX)
+ .append(batchNo)
+ .append("-")
+ .append(bucketNumber)
+ .append("-")
+ .append(segmentNo)
+ .append("-")
+ .append(factUpdateTimeStamp)
Review comment:
supposed to append compressor name here ? I see that argument `compressor` is not used.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-590183656
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2119/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3606: [CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#discussion_r382961565
##########
File path: streaming/src/main/java/org/apache/carbondata/streaming/CarbonStreamRecordWriter.java
##########
@@ -186,7 +188,7 @@ private void initializeAtFirstRow() throws IOException {
compressorName = carbonTable.getTableInfo().getFactTable().getTableProperties().get(
CarbonCommonConstants.COMPRESSOR);
if (null == compressorName) {
- compressorName = CompressorFactory.getInstance().getCompressor().getName();
+ compressorName = CompressorFactory.NativeSupportedCompressor.SNAPPY.getName();
Review comment:
revert this or use CCC default compressor
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#discussion_r383083186
##########
File path: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/describeTable/TestDescribeTable.scala
##########
@@ -68,15 +68,6 @@ class TestDescribeTable extends QueryTest with BeforeAndAfterAll {
assert(descPar.exists(_.toString().contains("Partition Parameters:")))
}
- test(testName = "Compressor Type update from carbon properties") {
- sql("drop table if exists b")
- sql(sqlText = "create table b(a int,b string) STORED AS carbondata")
- CarbonProperties.getInstance().addProperty(CarbonCommonConstants.COMPRESSOR, "gzip")
- val result = sql(sqlText = "desc formatted b").collect()
- assert(result.filter(row => row.getString(0).contains("Data File Compressor")).head.getString
Review comment:
No, this print is not correct. For example, when loading, user set to use ZSTD compressor, and then set to SNAPPY again after loading, then this command still prints SNAPPY which is misleading
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk edited a comment on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
jackylk edited a comment on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583959274
@QiangCai ok, I will verify on cluster, and with old table created before this PR
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk edited a comment on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
jackylk edited a comment on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583959274
@QiangCai ok, I will verify on cluster, and with old table
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file
name and change default compressor to ZSTD
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name and change default compressor to ZSTD
URL: https://github.com/apache/carbondata/pull/3606#discussion_r377419707
##########
File path: streaming/src/main/java/org/apache/carbondata/streaming/CarbonStreamRecordWriter.java
##########
@@ -140,7 +140,9 @@ private void initialize(TaskAttemptContext job) throws IOException {
segmentDir = CarbonTablePath.getSegmentPath(
carbonTable.getAbsoluteTableIdentifier().getTablePath(), segmentId);
- fileName = CarbonTablePath.getCarbonDataFileName(0, taskNo + "", 0, 0, "0", segmentId);
+ fileName = CarbonTablePath.getCarbonDataFileName(
Review comment:
ok, please add a TODO
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583764317
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1887/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606: [WIP] add
compressor to file name and change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [WIP] add compressor to file name and change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-582837564
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1868/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583311877
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1879/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3606: [CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r376989476
##########
File path: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/table/CarbonDescribeFormattedCommand.scala
##########
@@ -141,11 +141,6 @@ private[sql] case class CarbonDescribeFormattedCommand(
Strings.formatSize(
tblProps.getOrElse(CarbonCommonConstants.CARBON_LOAD_MIN_SIZE_INMB,
CarbonCommonConstants.CARBON_LOAD_MIN_SIZE_INMB_DEFAULT).toFloat), ""),
- ("Data File Compressor ", tblProps
Review comment:
why removed 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-583867782
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1902/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3606: [CARBONDATA-3688] Add compressor name in data file name
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3606: [CARBONDATA-3688] Add compressor name in data file name
URL: https://github.com/apache/carbondata/pull/3606#discussion_r382961494
##########
File path: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/table/CarbonDescribeFormattedCommand.scala
##########
@@ -141,11 +141,6 @@ private[sql] case class CarbonDescribeFormattedCommand(
Strings.formatSize(
tblProps.getOrElse(CarbonCommonConstants.CARBON_LOAD_MIN_SIZE_INMB,
CarbonCommonConstants.CARBON_LOAD_MIN_SIZE_INMB_DEFAULT).toFloat), ""),
- ("Data File Compressor ", tblProps
- .getOrElse(CarbonCommonConstants.COMPRESSOR,
- CarbonProperties.getInstance()
Review comment:
As now we are not changing the default compressor, we can revert this change ?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] ajantha-bhat commented on a change in pull request
#3606: [CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r376990677
##########
File path: streaming/src/main/java/org/apache/carbondata/streaming/CarbonStreamRecordWriter.java
##########
@@ -140,7 +140,9 @@ private void initialize(TaskAttemptContext job) throws IOException {
segmentDir = CarbonTablePath.getSegmentPath(
carbonTable.getAbsoluteTableIdentifier().getTablePath(), segmentId);
- fileName = CarbonTablePath.getCarbonDataFileName(0, taskNo + "", 0, 0, "0", segmentId);
+ fileName = CarbonTablePath.getCarbonDataFileName(
Review comment:
why always snappy for streaming segment ? before I think it was supporting configured compressor
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-584166330
Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/216/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] CarbonDataQA1 commented on issue #3606:
[CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name
and change default compressor to ZSTD
Posted by GitBox <gi...@apache.org>.
CarbonDataQA1 commented on issue #3606: [CARBONDATA-3681][CARBONDATA-3688] Add compressor name in data file name and change default compressor to ZSTD
URL: https://github.com/apache/carbondata/pull/3606#issuecomment-585156992
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1960/
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [carbondata] jackylk commented on a change in pull request #3606:
[CARBONDATA-3681] Change default compressor to zstd
Posted by GitBox <gi...@apache.org>.
jackylk commented on a change in pull request #3606: [CARBONDATA-3681] Change default compressor to zstd
URL: https://github.com/apache/carbondata/pull/3606#discussion_r376776809
##########
File path: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##########
@@ -1083,7 +1083,7 @@ private CarbonCommonConstants() {
* The optional values are 'SNAPPY','GZIP','BZIP2','LZ4','ZSTD' and empty.
* Specially, empty means that Carbondata will not compress the sort temp files.
*/
- public static final String CARBON_SORT_TEMP_COMPRESSOR_DEFAULT = "SNAPPY";
+ public static final String CARBON_SORT_TEMP_COMPRESSOR_DEFAULT = "zstd";
Review comment:
fixed
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services