[CARBONDATA-2804][DataMap] fix the bug when bloom filter or preaggregate datamap tried to be created on older V1-V2 version stores #2601
Conversation
|
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7731/ |
|
Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6457/ |
ndwangsen
force-pushed
the
fix_block_dm_v1_v2
branch
from
921c436
to
0cd355a
Compare
|
retest this please |
|
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7737/ |
| // get the carbon index file header | ||
| FileFactory.FileType fileType = FileFactory.getFileType(storePath); | ||
| ColumnarFormatVersion version = null; | ||
| if (FileFactory.isFileExist(storePath, fileType)) { |
There was a problem hiding this comment.
Rename storePath to segmentPath
There was a problem hiding this comment.
modified according to review comments
| indexReader.openThriftReader(fileData); | ||
| IndexHeader indexHeader = indexReader.readIndexHeader(); | ||
| version = ColumnarFormatVersion.valueOf((short)indexHeader.getVersion()); | ||
| break; |
There was a problem hiding this comment.
once reading is complete close indexReader using try and finally block
indexReader.closeThriftReader()
There was a problem hiding this comment.
modified according to review comments
| break; | ||
| } | ||
| } else { | ||
| version = CarbonProperties.getInstance().getFormatVersion(); |
There was a problem hiding this comment.
- This else condition is not required as for a valid segment its path will exist in the file system.
- If at all the path does not exist then read the version info from the next valid segment and log a warning that valid segment path does not exist in the system
There was a problem hiding this comment.
modified according to review comments
|
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6463/ |
|
hi, @ndwangsen Can you provide simple description about your solution in the PR description. |
|
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6122/ |
|
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6127/ |
tried to be created on older V1-V2 version stores
ndwangsen
force-pushed
the
fix_block_dm_v1_v2
branch
from
0cd355a
to
55f8fb4
Compare
|
retest this please |
|
Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6477/ |
|
retest this please |
|
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7753/ |
|
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6478/ |
|
retest this please |
|
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/7758/ |
|
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/6483/ |
|
LGTM |
1 similar comment
|
LGTM |
|
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/6139/ |
…ap tried to be created on older V1-V2 version stores This PR change read carbon file version from carbondata file header to carbonindex file header, because the version filed of carondata file header is not compatible with older v1/v2 version. This closes #2601
[CARBONDATA-2804] fix the bug when bloom filter or preaggregate datamap tried to be created on older V1-V2 version store
This PR change read carbon file version from carbondata file header to carbonindex file header, because
the version field of carbondata file header is not compatible with older v1/v2 version.
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
NA
NA
NA
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.
test pass in test environment
NA