You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "prashantwason (via GitHub)" <gi...@apache.org> on 2023/04/20 20:05:39 UTC
[GitHub] [hudi] prashantwason opened a new pull request, #8523: [HUDI-6114] Fixed rollback of blocks in scanInternalV1
prashantwason opened a new pull request, #8523:
URL: https://github.com/apache/hudi/pull/8523
[HUDI-6114] Fixed rollback of blocks in scanInternalV1
### Change Logs
Changed the rollback logic to consider all blocks rather than only the last block. This is correction in multi-writer usecases where other commits could have happened between the failed commit and the rollback block.
Added a unit test
### Impact
Log blocks will be correctly rolled back when using non optimized scanner and multi-writer
### Risk level (write none, low medium or high below)
None
### Documentation Update
None
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [ ] Change Logs and Impact were stated clearly
- [ ] Adequate tests were added if applicable
- [ ] CI passed
--
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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8523: [HUDI-6114] Fixed rollback of blocks in scanInternalV1
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8523:
URL: https://github.com/apache/hudi/pull/8523#issuecomment-1525735385
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16510",
"triggerID" : "8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212",
"triggerType" : "PUSH"
}, {
"hash" : "1450f0a041a5b2b5e8730ccc34c320ea7847abf6",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16702",
"triggerID" : "1450f0a041a5b2b5e8730ccc34c320ea7847abf6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 1450f0a041a5b2b5e8730ccc34c320ea7847abf6 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16702)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 merged pull request #8523: [HUDI-6114] Fixed rollback of blocks in scanInternalV1
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 merged PR #8523:
URL: https://github.com/apache/hudi/pull/8523
--
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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] prashantwason commented on a diff in pull request #8523: [HUDI-6114] Fixed rollback of blocks in scanInternalV1
Posted by "prashantwason (via GitHub)" <gi...@apache.org>.
prashantwason commented on code in PR #8523:
URL: https://github.com/apache/hudi/pull/8523#discussion_r1178663188
##########
hudi-common/src/test/java/org/apache/hudi/common/functional/TestHoodieLogFormat.java:
##########
@@ -2718,4 +2595,40 @@ private HoodieLogFormat.Reader createCorruptedFile(String fileId) throws Excepti
return reader;
}
+
+ private void checkLogBlocksAndKeys(String latestInstantTime, Schema schema, boolean readBlocksLazily,
+ ExternalSpillableMap.DiskMapType diskMapType, boolean isCompressionEnabled, boolean enableOptimizedLogBlocksScan, int expectedTotalRecords,
+ int expectedTotalKeys, Option<Set<String>> expectedKeys) throws IOException {
+ List<String> allLogFiles =
+ FSUtils.getAllLogFiles(fs, partitionPath, "test-fileid1", HoodieLogFile.DELTA_EXTENSION, "100")
+ .map(s -> s.getPath().toString()).collect(Collectors.toList());
+
+ HoodieMergedLogRecordScanner scanner = HoodieMergedLogRecordScanner.newBuilder()
+ .withFileSystem(fs)
+ .withBasePath(basePath)
+ .withLogFilePaths(allLogFiles)
+ .withReaderSchema(schema)
+ .withLatestInstantTime(latestInstantTime)
+ .withMaxMemorySizeInBytes(10240L)
+ .withReadBlocksLazily(readBlocksLazily)
+ .withReverseReader(false)
+ .withBufferSize(BUFFER_SIZE)
+ .withSpillableMapBasePath(spillableBasePath)
+ .withDiskMapType(diskMapType)
+ .withBitCaskDiskMapCompressionEnabled(isCompressionEnabled)
+ .withOptimizedLogBlocksScan(enableOptimizedLogBlocksScan)
+ .withRecordMerger(HoodieRecordUtils.loadRecordMerger(HoodieAvroRecordMerger.class.getName()))
+ .build();
+ assertEquals(expectedTotalRecords, scanner.getTotalLogRecords(), "There should be " + expectedTotalRecords + " records");
+ final Set<String> readKeys = new HashSet<>();
+ scanner.forEach(s -> readKeys.add(s.getKey().getRecordKey()));
+ assertEquals(expectedTotalKeys, readKeys.size(), "Read should return return all " + expectedTotalKeys + " keys");
+
+ if (expectedKeys.isPresent()) {
+ assertEquals(expectedKeys.get(), readKeys, "Keys read from log file should match written keys");
+ }
+ scanner.close();
Review Comment:
Enclosed in try
--
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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8523: [HUDI-6114] Fixed rollback of blocks in scanInternalV1
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8523:
URL: https://github.com/apache/hudi/pull/8523#issuecomment-1524858950
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16510",
"triggerID" : "8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212",
"triggerType" : "PUSH"
}, {
"hash" : "1450f0a041a5b2b5e8730ccc34c320ea7847abf6",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "1450f0a041a5b2b5e8730ccc34c320ea7847abf6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16510)
* 1450f0a041a5b2b5e8730ccc34c320ea7847abf6 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on a diff in pull request #8523: [HUDI-6114] Fixed rollback of blocks in scanInternalV1
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8523:
URL: https://github.com/apache/hudi/pull/8523#discussion_r1173268614
##########
hudi-common/src/test/java/org/apache/hudi/common/functional/TestHoodieLogFormat.java:
##########
@@ -2718,4 +2595,40 @@ private HoodieLogFormat.Reader createCorruptedFile(String fileId) throws Excepti
return reader;
}
+
+ private void checkLogBlocksAndKeys(String latestInstantTime, Schema schema, boolean readBlocksLazily,
+ ExternalSpillableMap.DiskMapType diskMapType, boolean isCompressionEnabled, boolean enableOptimizedLogBlocksScan, int expectedTotalRecords,
+ int expectedTotalKeys, Option<Set<String>> expectedKeys) throws IOException {
+ List<String> allLogFiles =
+ FSUtils.getAllLogFiles(fs, partitionPath, "test-fileid1", HoodieLogFile.DELTA_EXTENSION, "100")
+ .map(s -> s.getPath().toString()).collect(Collectors.toList());
+
+ HoodieMergedLogRecordScanner scanner = HoodieMergedLogRecordScanner.newBuilder()
+ .withFileSystem(fs)
+ .withBasePath(basePath)
+ .withLogFilePaths(allLogFiles)
+ .withReaderSchema(schema)
+ .withLatestInstantTime(latestInstantTime)
+ .withMaxMemorySizeInBytes(10240L)
+ .withReadBlocksLazily(readBlocksLazily)
+ .withReverseReader(false)
+ .withBufferSize(BUFFER_SIZE)
+ .withSpillableMapBasePath(spillableBasePath)
+ .withDiskMapType(diskMapType)
+ .withBitCaskDiskMapCompressionEnabled(isCompressionEnabled)
+ .withOptimizedLogBlocksScan(enableOptimizedLogBlocksScan)
+ .withRecordMerger(HoodieRecordUtils.loadRecordMerger(HoodieAvroRecordMerger.class.getName()))
+ .build();
+ assertEquals(expectedTotalRecords, scanner.getTotalLogRecords(), "There should be " + expectedTotalRecords + " records");
+ final Set<String> readKeys = new HashSet<>();
+ scanner.forEach(s -> readKeys.add(s.getKey().getRecordKey()));
+ assertEquals(expectedTotalKeys, readKeys.size(), "Read should return return all " + expectedTotalKeys + " keys");
+
+ if (expectedKeys.isPresent()) {
+ assertEquals(expectedKeys.get(), readKeys, "Keys read from log file should match written keys");
+ }
+ scanner.close();
Review Comment:
Why close 2 times here, we can use the `try ()` resource block instead.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8523: [HUDI-6114] Fixed rollback of blocks in scanInternalV1
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8523:
URL: https://github.com/apache/hudi/pull/8523#issuecomment-1516931783
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8523: [HUDI-6114] Fixed rollback of blocks in scanInternalV1
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8523:
URL: https://github.com/apache/hudi/pull/8523#issuecomment-1517423691
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16510",
"triggerID" : "8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16510)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8523: [HUDI-6114] Fixed rollback of blocks in scanInternalV1
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8523:
URL: https://github.com/apache/hudi/pull/8523#issuecomment-1524876144
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16510",
"triggerID" : "8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212",
"triggerType" : "PUSH"
}, {
"hash" : "1450f0a041a5b2b5e8730ccc34c320ea7847abf6",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16702",
"triggerID" : "1450f0a041a5b2b5e8730ccc34c320ea7847abf6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16510)
* 1450f0a041a5b2b5e8730ccc34c320ea7847abf6 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16702)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on a diff in pull request #8523: [HUDI-6114] Fixed rollback of blocks in scanInternalV1
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8523:
URL: https://github.com/apache/hudi/pull/8523#discussion_r1173263330
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/AbstractHoodieLogRecordReader.java:
##########
@@ -306,45 +306,43 @@ private void scanInternalV1(Option<KeySpec> keySpecOpt) {
// written per ingestion batch for a file but in reality we need to rollback (B1 & B2)
// The following code ensures the same rollback block (R1) is used to rollback
// both B1 & B2
- LOG.info("Reading a command block from file " + logFile.getPath());
// This is a command block - take appropriate action based on the command
HoodieCommandBlock commandBlock = (HoodieCommandBlock) logBlock;
String targetInstantForCommandBlock =
logBlock.getLogBlockHeader().get(HoodieLogBlock.HeaderMetadataType.TARGET_INSTANT_TIME);
+ LOG.info(String.format("Reading a command block %s with targetInstantTime %s from file %s", commandBlock.getType(), targetInstantForCommandBlock,
+ logFile.getPath()));
switch (commandBlock.getType()) { // there can be different types of command blocks
case ROLLBACK_BLOCK:
- // Rollback the last read log block
- // Get commit time from last record block, compare with targetCommitTime,
+ // Rollback older read log block(s)
+ // Get commit time from older record blocks, compare with targetCommitTime,
// rollback only if equal, this is required in scenarios of invalid/extra
// rollback blocks written due to failures during the rollback operation itself
// and ensures the same rollback block (R1) is used to rollback both B1 & B2 with
- // same instant_time
- int numBlocksRolledBack = 0;
- totalRollbacks.incrementAndGet();
- while (!currentInstantLogBlocks.isEmpty()) {
- HoodieLogBlock lastBlock = currentInstantLogBlocks.peek();
+ // same instant_time.
+ final int instantLogBlockSizeBeforeRollback = currentInstantLogBlocks.size();
+ currentInstantLogBlocks.removeIf(lastBlock -> {
// handle corrupt blocks separately since they may not have metadata
if (lastBlock.getBlockType() == CORRUPT_BLOCK) {
Review Comment:
lastBlock -> block ? Because it may not be the last one.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on pull request #8523: [HUDI-6114] Fixed rollback of blocks in scanInternalV1
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on PR #8523:
URL: https://github.com/apache/hudi/pull/8523#issuecomment-1526936486
The failed test: https://dev.azure.com/apache-hudi-ci-org/apache-hudi-ci/_build/results?buildId=16702&view=logs&j=3b6e910d-b98f-5de6-b9cb-1e5ff571f5de&t=30b5aae4-0ea0-5566-42d0-febf71a7061a&l=717193
is a known falky test that has been fixed on master.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8523: [HUDI-6114] Fixed rollback of blocks in scanInternalV1
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8523:
URL: https://github.com/apache/hudi/pull/8523#issuecomment-1516942596
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16510",
"triggerID" : "8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8b2fbfcf5edcb39d36ccbc41bc2b30c2e8aa4212 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16510)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] prashantwason commented on a diff in pull request #8523: [HUDI-6114] Fixed rollback of blocks in scanInternalV1
Posted by "prashantwason (via GitHub)" <gi...@apache.org>.
prashantwason commented on code in PR #8523:
URL: https://github.com/apache/hudi/pull/8523#discussion_r1178653357
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/AbstractHoodieLogRecordReader.java:
##########
@@ -306,45 +306,43 @@ private void scanInternalV1(Option<KeySpec> keySpecOpt) {
// written per ingestion batch for a file but in reality we need to rollback (B1 & B2)
// The following code ensures the same rollback block (R1) is used to rollback
// both B1 & B2
- LOG.info("Reading a command block from file " + logFile.getPath());
// This is a command block - take appropriate action based on the command
HoodieCommandBlock commandBlock = (HoodieCommandBlock) logBlock;
String targetInstantForCommandBlock =
logBlock.getLogBlockHeader().get(HoodieLogBlock.HeaderMetadataType.TARGET_INSTANT_TIME);
+ LOG.info(String.format("Reading a command block %s with targetInstantTime %s from file %s", commandBlock.getType(), targetInstantForCommandBlock,
+ logFile.getPath()));
switch (commandBlock.getType()) { // there can be different types of command blocks
case ROLLBACK_BLOCK:
- // Rollback the last read log block
- // Get commit time from last record block, compare with targetCommitTime,
+ // Rollback older read log block(s)
+ // Get commit time from older record blocks, compare with targetCommitTime,
// rollback only if equal, this is required in scenarios of invalid/extra
// rollback blocks written due to failures during the rollback operation itself
// and ensures the same rollback block (R1) is used to rollback both B1 & B2 with
- // same instant_time
- int numBlocksRolledBack = 0;
- totalRollbacks.incrementAndGet();
- while (!currentInstantLogBlocks.isEmpty()) {
- HoodieLogBlock lastBlock = currentInstantLogBlocks.peek();
+ // same instant_time.
+ final int instantLogBlockSizeBeforeRollback = currentInstantLogBlocks.size();
+ currentInstantLogBlocks.removeIf(lastBlock -> {
// handle corrupt blocks separately since they may not have metadata
if (lastBlock.getBlockType() == CORRUPT_BLOCK) {
Review Comment:
Corrected.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org