Skip to content

[HUDI-4279] Strength the remote fs view lagging check when latest com… #5917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danny0405 merged 1 commit into from
Jun 22, 2022
Merged

[HUDI-4279] Strength the remote fs view lagging check when latest com… #5917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ...mmon/src/main/java/org/apache/hudi/common/table/view/RemoteHoodieTableFileSystemView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ public class RemoteHoodieTableFileSystemView implements SyncableFileSystemView,
public static final String FILEID_PARAM = "fileid";
public static final String LAST_INSTANT_TS = "lastinstantts";
public static final String TIMELINE_HASH = "timelinehash";
public static final String NUM_INSTANTS = "numinstants";
public static final String REFRESH_OFF = "refreshoff";
public static final String INCLUDE_FILES_IN_PENDING_COMPACTION_PARAM = "includependingcompaction";

Expand Down Expand Up @@ -162,6 +163,7 @@ private <T> T executeRequest(String requestPath, Map<String, String> queryParame
// Adding mandatory parameters - Last instants affecting file-slice
timeline.lastInstant().ifPresent(instant -> builder.addParameter(LAST_INSTANT_TS, instant.getTimestamp()));
builder.addParameter(TIMELINE_HASH, timeline.getTimelineHash());
builder.addParameter(NUM_INSTANTS, timeline.countInstants() + "");

String url = builder.toString();
LOG.info("Sending request : (" + url + ")");
Expand Down
22 changes: 19 additions & 3 deletions hudi-timeline-service/src/main/java/org/apache/hudi/timeline/service/RequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,9 @@ private boolean isLocalViewBehind(Context ctx) {
String lastKnownInstantFromClient =
ctx.queryParam(RemoteHoodieTableFileSystemView.LAST_INSTANT_TS, HoodieTimeline.INVALID_INSTANT_TS);
String timelineHashFromClient = ctx.queryParam(RemoteHoodieTableFileSystemView.TIMELINE_HASH, "");
String numInstantsFromClient = ctx.queryParam(RemoteHoodieTableFileSystemView.NUM_INSTANTS, "-1");
HoodieTimeline localTimeline =
viewManager.getFileSystemView(basePath).getTimeline().filterCompletedAndCompactionInstants();
String localLastKnownInstant = localTimeline.lastInstant().isPresent() ? localTimeline.lastInstant().get().getTimestamp()
: HoodieTimeline.INVALID_INSTANT_TS;
if (LOG.isDebugEnabled()) {
LOG.debug("Client [ LastTs=" + lastKnownInstantFromClient + ", TimelineHash=" + timelineHashFromClient
+ "], localTimeline=" + localTimeline.getInstants().collect(Collectors.toList()));
Expand All @@ -138,14 +137,31 @@ private boolean isLocalViewBehind(Context ctx) {
String localTimelineHash = localTimeline.getTimelineHash();
// refresh if timeline hash mismatches and if local's last known instant < client's last known instant (if config is enabled)
if (!localTimelineHash.equals(timelineHashFromClient)
&& (!timelineServiceConfig.refreshTimelineBasedOnLatestCommit || HoodieTimeline.compareTimestamps(localLastKnownInstant, HoodieTimeline.LESSER_THAN, lastKnownInstantFromClient))) {
&& (!timelineServiceConfig.refreshTimelineBasedOnLatestCommit
|| localTimelineBehind(localTimeline, lastKnownInstantFromClient, numInstantsFromClient))) {
return true;
}

// As a safety check, even if hash is same, ensure instant is present
return !localTimeline.containsOrBeforeTimelineStarts(lastKnownInstantFromClient);
}

private static boolean localTimelineBehind(HoodieTimeline localTimeline, String lastKnownInstantFromClient, String numInstantsFromClient) {
String localLastKnownInstant = localTimeline.lastInstant().isPresent() ? localTimeline.lastInstant().get().getTimestamp()
: HoodieTimeline.INVALID_INSTANT_TS;
// Why comparing the num commits ?
// Assumes there are 4 commits on the timeline:
// timestamp(action): ts_0(commit), ts_1(commit), ts_2(clean), ts_3(commit)
// when ts_1 is in INFLIGHT state, ts_2 clean action is already finished,
// after ts_1 triggers #sync, the local timeline is refreshed as [ts_0, ts_2],
// when ts_1 switches state from INFLIGHT to COMPLETED, no #sync triggers.
// at ts_3, when the fs view snapshot is requested, the ts_3 client timeline should be [ts_0, ts_1, ts_2],
// if we only compare the latest commit, the local timeline is NOT behind, but the fs view is not complete
// because ts_1 is lost.
return HoodieTimeline.compareTimestamps(localLastKnownInstant, HoodieTimeline.LESSER_THAN, lastKnownInstantFromClient)
|| localTimeline.countInstants() < Integer.parseInt(numInstantsFromClient);
Copy link
Contributor

@nsivabalan nsivabalan Jun 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we check just "<" or "!=" to catch any mismatch ?

Copy link
Contributor

@danny0405 danny0405 Jun 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can i guess, but keeping

HoodieTimeline.compareTimestamps(localLastKnownInstant, HoodieTimeline.LESSER_THAN, lastKnownInstantFromClient)

as a fast check is fine i think.

}

/**
* Syncs data-set view if local view is behind.
*/
Expand Down