[HUDI-3194] fix MOR snapshot query (HIVE) during compaction

[YuweiXiao]

Tips

• Thank you very much for contributing to Apache Hudi.
• Please review https://hudi.apache.org/contribute/how-to-contribute before opening a pull request.

What is the purpose of the pull request

Fix MOR snapshot query path during compaction for HIVE read.

In current implementation, if a write comes in and complete during compaction, it will not be visible to snapshot query until the compaction completes. This is caused by filter logic of getting file group's log files.

Brief change log

• Include pending compactions in the timeline for the file group retrieval logic.

Verify this pull request

This change added tests and can be verified as follows:

• Add functional test about snapshot query during compaction.

Committer checklist

• Has a corresponding JIRA in PR title & commit

• Commit message is descriptive of the change

• CI is green

• Necessary doc changes done or have another open PR

• For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

[nsivabalan]

@YuweiXiao : I see "WIP" in title. Is the patch good to review or is it still being worked upon ?

[YuweiXiao]

@YuweiXiao : I see "WIP" in title. Is the patch good to review or is it still being worked upon ?

Yes, it is still in process, as there are failed UT need to be fixed.

[nsivabalan]

@xiarixiaoyao : hey, can you review this patch please. Touches part of the code authored by you.

[hudi-bot]

CI report:

• 49b0796 Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

• @hudi-bot run azure re-run the last Azure build

[codope]

@YuweiXiao Why not use BaseFileWithLogsSplit which maps matching log files to base files?

[codope]

The writeTimeline will also contain the compaction instant compared to commitsTimeline, but how does that matter for this scenario? Since latest active timeline is already being passed to createInMemoryFileSystemViewWithTimeline then latest file slice would contain the file group due to commit during ongoing compaction right?

[YuweiXiao]

It won't affect the correctness. The latestCompletedInstant is used to filter file slice. Considering a compaction only case, without including the completed compaction instant, we will end up reading 'old version' file slice (i.e., base file + log) rather than the compacted one (i.e., only base file, which has better performance).

[nsivabalan]

I also don't understand the fix. can you help throw some light. From the description in this patch, the gap is, when compaction is on-going and a new write comes in and completes, it may not be visible to queries.
But the fix here, just includes compaction instants to the list of instants to process. Not sure if the description matches the fix.
or am I missing anything here.

[YuweiXiao]

I also don't understand the fix. can you help throw some light. From the description in this patch, the gap is, when compaction is on-going and a new write comes in and completes, it may not be visible to queries. But the fix here, just includes compaction instants to the list of instants to process. Not sure if the description matches the fix. or am I missing anything here.

Hey! In fsView::getLatestMergedFileSlicesBeforeOrOn, there is a logic where we check if a file group is under compaction (under construction), so that we could add logs files generated by concurrent writers. And only passing a timeline including compactions, this logic could work (fsView::fetchMergedFileSlice).

[nsivabalan]

got it. thanks for explaining in detail. Fix makes sense then.

[YuweiXiao]

@YuweiXiao Why not use BaseFileWithLogsSplit which maps matching log files to base files?

Actually, I tried with BaseFileWithLogsSplit. IIUC, we need to use BaseFileWithLogsSplit at the place where we generate the split, i.e., HoodieInputFormatUtils::filterFileStatusForSnapshotMode. By looking at its implementation, I found its semantic is to generate baseFile split and log file only file slice. And callers of this function rely on this semantic, such as bootstrap.

[xiarixiaoyao]

@YuweiXiao only a small question: Does clustering also have this problem？

[YuweiXiao]

@YuweiXiao only a small question: Does clustering also have this problem？

I guess not. Currently, clustering doesn't support concurrent updates. So there won't be new log files when doing the clustering.

But you remind me it may be a problem in the future. I am working on consistent hashing index recently, which could enhance clustering to support concurrent update. I will keep an eye on it.

[xiarixiaoyao]

LGTM

[xiarixiaoyao]

@codope could you pls review this pr again, thanks