Skip to content

[HUDI-7163] Fix not parsable text DateTimeParseException when compact #10220

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
yihua merged 1 commit into from
Mar 9, 2024
Merged

[HUDI-7163] Fix not parsable text DateTimeParseException when compact #10220

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
10 changes: 2 additions & 8 deletions .../src/main/java/org/apache/hudi/table/action/compact/ScheduleCompactionActionExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import javax.annotation.Nullable;

import java.io.IOException;
import java.text.ParseException;
import java.util.Map;

import static org.apache.hudi.common.util.CollectionUtils.nonEmpty;
Expand Down Expand Up @@ -211,12 +210,7 @@ private boolean needCompact(CompactionTriggerStrategy compactionTriggerStrategy)
}

private Long parsedToSeconds(String time) {
long timestamp;
try {
timestamp = HoodieActiveTimeline.parseDateFromInstantTime(time).getTime() / 1000;
} catch (ParseException e) {
throw new HoodieCompactionException(e.getMessage(), e);
}
return timestamp;
return HoodieActiveTimeline.parseDateFromInstantTimeSafely(time).orElseThrow(() -> new HoodieCompactionException("Failed to parse timestamp " + time))
.getTime() / 1000;
}
}
39 changes: 17 additions & 22 deletions hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/util/StreamerUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
import java.io.BufferedReader;
import java.io.IOException;
import java.io.StringReader;
import java.text.ParseException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
Expand Down Expand Up @@ -380,34 +379,30 @@ public static Option<HoodieTableConfig> getTableConfig(String basePath, org.apac
* Returns the median instant time between the given two instant time.
*/
public static Option<String> medianInstantTime(String highVal, String lowVal) {
try {
long high = HoodieActiveTimeline.parseDateFromInstantTime(highVal).getTime();
long low = HoodieActiveTimeline.parseDateFromInstantTime(lowVal).getTime();
ValidationUtils.checkArgument(high > low,
"Instant [" + highVal + "] should have newer timestamp than instant [" + lowVal + "]");
long median = low + (high - low) / 2;
final String instantTime = HoodieActiveTimeline.formatDate(new Date(median));
if (HoodieTimeline.compareTimestamps(lowVal, HoodieTimeline.GREATER_THAN_OR_EQUALS, instantTime)
|| HoodieTimeline.compareTimestamps(highVal, HoodieTimeline.LESSER_THAN_OR_EQUALS, instantTime)) {
return Option.empty();
}
return Option.of(instantTime);
} catch (ParseException e) {
throw new HoodieException("Get median instant time with interval [" + lowVal + ", " + highVal + "] error", e);
long high = HoodieActiveTimeline.parseDateFromInstantTimeSafely(highVal)
.orElseThrow(() -> new HoodieException("Get instant time diff with interval [" + highVal + "] error")).getTime();
long low = HoodieActiveTimeline.parseDateFromInstantTimeSafely(lowVal)
Copy link
Contributor

@danny0405 danny0405 Dec 14, 2023

Choose a reason for hiding this comment

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

Does the exception only happens for table upgrade?

Copy link
Contributor Author

@wuzhenhua01 wuzhenhua01 Dec 14, 2023

Choose a reason for hiding this comment

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

it is mainly fordefensive programming

Copy link
Contributor

@danny0405 danny0405 Dec 14, 2023

Choose a reason for hiding this comment

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

Do you encounter exceptions in production?

Copy link
Contributor Author

@wuzhenhua01 wuzhenhua01 Dec 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

i mean the method orElseThrow is fordefensive programming. sure, we can repeat the provided operation procedure to reproduce this exception

Copy link
Contributor

@danny0405 danny0405 Dec 14, 2023

Choose a reason for hiding this comment

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

Okay, if we are not enocunting problems, would de-prioritize the PR a little bit.

.orElseThrow(() -> new HoodieException("Get instant time diff with interval [" + lowVal + "] error")).getTime();
ValidationUtils.checkArgument(high > low,
"Instant [" + highVal + "] should have newer timestamp than instant [" + lowVal + "]");
long median = low + (high - low) / 2;
final String instantTime = HoodieActiveTimeline.formatDate(new Date(median));
if (HoodieTimeline.compareTimestamps(lowVal, HoodieTimeline.GREATER_THAN_OR_EQUALS, instantTime)
|| HoodieTimeline.compareTimestamps(highVal, HoodieTimeline.LESSER_THAN_OR_EQUALS, instantTime)) {
return Option.empty();
}
return Option.of(instantTime);
}

/**
* Returns the time interval in seconds between the given instant time.
*/
public static long instantTimeDiffSeconds(String newInstantTime, String oldInstantTime) {
try {
long newTimestamp = HoodieActiveTimeline.parseDateFromInstantTime(newInstantTime).getTime();
long oldTimestamp = HoodieActiveTimeline.parseDateFromInstantTime(oldInstantTime).getTime();
return (newTimestamp - oldTimestamp) / 1000;
} catch (ParseException e) {
throw new HoodieException("Get instant time diff with interval [" + oldInstantTime + ", " + newInstantTime + "] error", e);
}
long newTimestamp = HoodieActiveTimeline.parseDateFromInstantTimeSafely(newInstantTime)
.orElseThrow(() -> new HoodieException("Get instant time diff with interval [" + oldInstantTime + ", " + newInstantTime + "] error")).getTime();
long oldTimestamp = HoodieActiveTimeline.parseDateFromInstantTimeSafely(oldInstantTime)
.orElseThrow(() -> new HoodieException("Get instant time diff with interval [" + oldInstantTime + ", " + newInstantTime + "] error")).getTime();
return (newTimestamp - oldTimestamp) / 1000;
}

public static Option<Transformer> createTransformer(List<String> classNames) throws IOException {
Expand Down