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/28 23:35:36 UTC
[GitHub] [hudi] prashantwason opened a new pull request, #8607: [MINOR] Fixed the reading of instants from very old archive files where ACTION_STATE is not present in instants.
prashantwason opened a new pull request, #8607:
URL: https://github.com/apache/hudi/pull/8607
[MINOR] Fixed the reading of instants from very old archive files where ACTION_STATE is not present in instants.
### Change Logs
Ensure that ACTION_STATE exists in the record before performing the equals comparison.
### Impact
Very old archive files can be read.
### 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] danny0405 commented on a diff in pull request #8607: [MINOR] Fixed the reading of instants from very old archive files where ACTION_STATE is not present in instants.
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8607:
URL: https://github.com/apache/hudi/pull/8607#discussion_r1189293323
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieArchivedTimeline.java:
##########
@@ -152,9 +156,13 @@ public void loadCompactionDetailsInMemory(String compactionInstantTime) {
public void loadCompactionDetailsInMemory(String startTs, String endTs) {
// load compactionPlan
- loadInstants(new TimeRangeFilter(startTs, endTs), true, record ->
- record.get(ACTION_TYPE_KEY).toString().equals(HoodieTimeline.COMPACTION_ACTION)
- && HoodieInstant.State.INFLIGHT.toString().equals(record.get(ACTION_STATE).toString())
+ loadInstants(new TimeRangeFilter(startTs, endTs), true,
+ record -> {
+ // Older files don't have action state set.
+ Object action = record.get(ACTION_STATE);
+ return record.get(ACTION_TYPE_KEY).toString().equals(HoodieTimeline.COMPACTION_ACTION)
+ && (action == null || HoodieInstant.State.INFLIGHT.toString().equals(action.toString()));
Review Comment:
Fine, we are good to go, I did see several issues with this in-compatibility, let's merge it first.
--
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 #8607: [MINOR] Fixed the reading of instants from very old archive files where ACTION_STATE is not present in instants.
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8607:
URL: https://github.com/apache/hudi/pull/8607#issuecomment-1528225632
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "18ec6f29e045dbb17ba587b54279b807492f71f0",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "18ec6f29e045dbb17ba587b54279b807492f71f0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 18ec6f29e045dbb17ba587b54279b807492f71f0 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] prashantwason commented on a diff in pull request #8607: [MINOR] Fixed the reading of instants from very old archive files where ACTION_STATE is not present in instants.
Posted by "prashantwason (via GitHub)" <gi...@apache.org>.
prashantwason commented on code in PR #8607:
URL: https://github.com/apache/hudi/pull/8607#discussion_r1188645068
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieArchivedTimeline.java:
##########
@@ -152,9 +156,13 @@ public void loadCompactionDetailsInMemory(String compactionInstantTime) {
public void loadCompactionDetailsInMemory(String startTs, String endTs) {
// load compactionPlan
- loadInstants(new TimeRangeFilter(startTs, endTs), true, record ->
- record.get(ACTION_TYPE_KEY).toString().equals(HoodieTimeline.COMPACTION_ACTION)
- && HoodieInstant.State.INFLIGHT.toString().equals(record.get(ACTION_STATE).toString())
+ loadInstants(new TimeRangeFilter(startTs, endTs), true,
+ record -> {
+ // Older files don't have action state set.
+ Object action = record.get(ACTION_STATE);
+ return record.get(ACTION_TYPE_KEY).toString().equals(HoodieTimeline.COMPACTION_ACTION)
+ && (action == null || HoodieInstant.State.INFLIGHT.toString().equals(action.toString()));
Review Comment:
old version is deprecated/fixed so dont know how to write unit tests for them.
--
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 #8607: [HUDI-6196] Keep compatibility for old version archival instants without ACTION_STATE field
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 merged PR #8607:
URL: https://github.com/apache/hudi/pull/8607
--
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 #8607: [MINOR] Fixed the reading of instants from very old archive files where ACTION_STATE is not present in instants.
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8607:
URL: https://github.com/apache/hudi/pull/8607#issuecomment-1528307185
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "18ec6f29e045dbb17ba587b54279b807492f71f0",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16751",
"triggerID" : "18ec6f29e045dbb17ba587b54279b807492f71f0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 18ec6f29e045dbb17ba587b54279b807492f71f0 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16751)
<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 #8607: [MINOR] Fixed the reading of instants from very old archive files where ACTION_STATE is not present in instants.
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8607:
URL: https://github.com/apache/hudi/pull/8607#discussion_r1181156586
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieArchivedTimeline.java:
##########
@@ -152,9 +156,13 @@ public void loadCompactionDetailsInMemory(String compactionInstantTime) {
public void loadCompactionDetailsInMemory(String startTs, String endTs) {
// load compactionPlan
- loadInstants(new TimeRangeFilter(startTs, endTs), true, record ->
- record.get(ACTION_TYPE_KEY).toString().equals(HoodieTimeline.COMPACTION_ACTION)
- && HoodieInstant.State.INFLIGHT.toString().equals(record.get(ACTION_STATE).toString())
+ loadInstants(new TimeRangeFilter(startTs, endTs), true,
+ record -> {
+ // Older files don't have action state set.
+ Object action = record.get(ACTION_STATE);
+ return record.get(ACTION_TYPE_KEY).toString().equals(HoodieTimeline.COMPACTION_ACTION)
+ && (action == null || HoodieInstant.State.INFLIGHT.toString().equals(action.toString()));
Review Comment:
When action equals null, the instant state is definite to be `INFLIGHT` for old version ? Can we write ta test case?
##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieArchivedTimeline.java:
##########
@@ -143,7 +143,11 @@ public void loadInstantDetailsInMemory(String startTs, String endTs) {
public void loadCompletedInstantDetailsInMemory() {
loadInstants(null, true,
- record -> HoodieInstant.State.COMPLETED.toString().equals(record.get(ACTION_STATE).toString()));
+ record -> {
+ // Very old archived instants don't have action state set.
+ Object action = record.get(ACTION_STATE);
+ return action == null || HoodieInstant.State.COMPLETED.toString().equals(action.toString());
Review Comment:
When action equals null, the instant state is definite to be `COMPLETE` for old version ? Can we write ta test case?
--
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 #8607: [MINOR] Fixed the reading of instants from very old archive files where ACTION_STATE is not present in instants.
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8607:
URL: https://github.com/apache/hudi/pull/8607#issuecomment-1528718856
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "18ec6f29e045dbb17ba587b54279b807492f71f0",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16751",
"triggerID" : "18ec6f29e045dbb17ba587b54279b807492f71f0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 18ec6f29e045dbb17ba587b54279b807492f71f0 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=16751)
<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