You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "codope (via GitHub)" <gi...@apache.org> on 2025/04/19 13:53:53 UTC
[PR] [HUDI-9325] Fix HFile seek logic to allow sequential non‐existent key lookups within a block [hudi]
codope opened a new pull request, #13177:
URL: https://github.com/apache/hudi/pull/13177
### Change Logs
When performing random access lookups against an HFile, callers often do:
1. seekTo() to position at the first record at the start of the file.
2. seekTo(key) to floor or find an exact match.
3. examine the return code (SEEK_TO_FOUND / SEEK_TO_IN_RANGE / SEEK_TO_BEFORE_FIRST_KEY / SEEK_TO_EOF).
However, if you issue two non‐existent lookups in a row, and the second lookup is for a key that is lexicographically smaller than the key where cursor is pointed after the first lookup, then the second seekTo(key) would throw an exception:
```
Caused by: java.lang.IllegalStateException: The current lookup key is less than the current position of the cursor, i.e., backward seekTo, which is not supported and should be avoided. key=UTF8StringKey{aZud4Oj9wxA=AZcHyzibzy8=bJfAFKIt2udY/eEKkDkFbg==} cursor=HFilePosition{offset=10503431, keyValue=Option{val=KeyValue{key=Key{aZud4Oj9wxA=AZcHyzibzy8=bLXWAR+d0ui0DMuh0uAXbw==}}}}
at org.apache.hudi.io.hfile.HFileReaderImpl.seekTo(HFileReaderImpl.java:173)
at org.apache.hudi.io.storage.HoodieNativeAvroHFileReader$RecordByKeyIterator.hasNext(HoodieNativeAvroHFileReader.java:404)
...
```
That happened because we only treated "backward" relative to the very first block in the file, not relative to whichever block we’d just loaded via `floorEntry(key)`.
This PR fixes the check for `isAtFirstKey` (now renamed to `isAtBlockStart`) to also consider whether the cursor’s offset equals the start of the _**current**_ data block.
So, the changed behavior is as follows;
- Lookups < block’s first key now yield SEEK_TO_BEFORE_FIRST_KEY instead of exception.
- All other semantics (exact match, in‐range, beyond‐last => IN_RANGE or EOF as before) remain unchanged.
- No additional I/O overhead; only a small offset comparison in the guard.
### Impact
Downstream readers (e.g., RecordByKeyIterator for index lookups) now safely handle arbitrary lookup without reader re‐initialization or catch‐and‐reset hacks.
### Risk level (write none, low medium or high below)
low
### Documentation Update
_Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none"._
- _The config description must be updated if new configs are added or the default value of the configs are changed_
- _Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
ticket number here and follow the [instruction](https://hudi.apache.org/contribute/developer-setup#website) to make
changes to the website._
### 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
Re: [PR] [HUDI-9325] Fix HFile seek logic to allow sequential non‐existent key lookups within a block [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13177:
URL: https://github.com/apache/hudi/pull/13177#issuecomment-2816717979
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a0475cc56c2702256f08289e32d27e1f17373fe7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a0475cc56c2702256f08289e32d27e1f17373fe7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a0475cc56c2702256f08289e32d27e1f17373fe7 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
Re: [PR] [HUDI-9325] Fix HFile seek logic to allow sequential non‐existent key lookups within a block [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope closed pull request #13177: [HUDI-9325] Fix HFile seek logic to allow sequential non‐existent key lookups within a block
URL: https://github.com/apache/hudi/pull/13177
--
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
Re: [PR] [HUDI-9325] Fix HFile seek logic to allow sequential non‐existent key lookups within a block [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #13177:
URL: https://github.com/apache/hudi/pull/13177#discussion_r2051528480
##########
hudi-io/src/test/java/org/apache/hudi/io/hfile/TestHFileReader.java:
##########
@@ -268,6 +271,33 @@ public void testReadHFilePointAndPrefixLookup(String filename,
verifyHFileRead(filename, numEntries, KEY_CREATOR, VALUE_CREATOR, keyLookUpInfoList);
}
+ @Test
+ public void testReadHFileCorrupted() throws Exception {
+ List<String> keys = new ArrayList<>();
+ try (HFileReader reader = getHFileReader("/hfile/col-stats-0001-0_1-170431-5847084_20250418193125815.hfile")) {
+ reader.initializeMetadata();
+ boolean available = reader.seekTo();
+ while (available) {
+ keys.add(reader.getKeyValue().get().getKey().getContentInString());
+ available = reader.next();
+ }
+ }
+ List<String> sortedKeys = new ArrayList<>(keys);
+ Collections.sort(sortedKeys);
+ assertEquals(sortedKeys, keys);
+ }
+
+ @Test
+ public void testSeekKeyDoesNotExist() throws Exception {
Review Comment:
Could the new lookup key case added to `testArgsReadHFilePointAndPrefixLookup` using existing test fixtures instead of adding a test method?
--
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
Re: [PR] [HUDI-9325] Fix HFile seek logic to allow sequential non‐existent key lookups within a block [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13177:
URL: https://github.com/apache/hudi/pull/13177#issuecomment-2816755646
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a0475cc56c2702256f08289e32d27e1f17373fe7",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=4968",
"triggerID" : "a0475cc56c2702256f08289e32d27e1f17373fe7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a0475cc56c2702256f08289e32d27e1f17373fe7 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=4968)
<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
Re: [PR] [HUDI-9325] Fix HFile seek logic to allow sequential non‐existent key lookups within a block [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on PR #13177:
URL: https://github.com/apache/hudi/pull/13177#issuecomment-2817489644
Closing in favor of #13179
--
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
Re: [PR] [HUDI-9325] Fix HFile seek logic to allow sequential non‐existent key lookups within a block [hudi]
Posted by "vinishjail97 (via GitHub)" <gi...@apache.org>.
vinishjail97 commented on code in PR #13177:
URL: https://github.com/apache/hudi/pull/13177#discussion_r2051517119
##########
hudi-io/src/test/java/org/apache/hudi/io/hfile/TestHFileReader.java:
##########
@@ -268,6 +271,33 @@ public void testReadHFilePointAndPrefixLookup(String filename,
verifyHFileRead(filename, numEntries, KEY_CREATOR, VALUE_CREATOR, keyLookUpInfoList);
}
+ @Test
+ public void testReadHFileCorrupted() throws Exception {
Review Comment:
Do we need this unit test ? We can move the validation of the HFile as an assertion in `testSeekKeyDoesNotExist` or remove it completely ?
--
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
Re: [PR] [HUDI-9325] Fix HFile seek logic to allow sequential non‐existent key lookups within a block [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #13177:
URL: https://github.com/apache/hudi/pull/13177#issuecomment-2816718426
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a0475cc56c2702256f08289e32d27e1f17373fe7",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=4968",
"triggerID" : "a0475cc56c2702256f08289e32d27e1f17373fe7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a0475cc56c2702256f08289e32d27e1f17373fe7 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=4968)
<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
Re: [PR] [HUDI-9325] Fix HFile seek logic to allow sequential non‐existent key lookups within a block [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on PR #13177:
URL: https://github.com/apache/hudi/pull/13177#issuecomment-2816765806
lets see if we can reproduce w/ simple UT where we write the hfile and trigger reads to reproduce the issue.
--
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