Skip to content
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.

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

[HUDI-5823] Partition ttl management #9723

Merged
merged 4 commits into from
Feb 28, 2024

Conversation

stream2000
Copy link
Contributor

@stream2000 stream2000 commented Sep 15, 2023

Change Logs

Poc code for Partition ttl management

Impact

NONE

Risk level (write none, low medium or high below)

NONE

Documentation Update

NONE

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

@stream2000 stream2000 force-pushed the partition-ttl-management-poc branch 3 times, most recently from 59df678 to 8081b29 Compare November 1, 2023 02:06
@stream2000 stream2000 changed the title [DO NOT MERGE][HUDI-5823]Poc code for Partition ttl management [HUDI-5823][DNM]Poc code for Partition ttl management Nov 1, 2023
@stream2000 stream2000 force-pushed the partition-ttl-management-poc branch 4 times, most recently from b81e894 to 2c1e5f6 Compare November 7, 2023 09:47
@stream2000 stream2000 changed the title [HUDI-5823][DNM]Poc code for Partition ttl management [HUDI-5823]Poc code for Partition ttl management Nov 17, 2023
@zyclove
Copy link

zyclove commented Nov 26, 2023

Will version 1.0 support this feature? This feature is greatly needed.
@stream2000 Thanks.👍🏻

@stream2000 stream2000 marked this pull request as draft December 27, 2023 13:05
@stream2000 stream2000 changed the title [HUDI-5823]Poc code for Partition ttl management [HUDI-5823] Partition ttl management Dec 27, 2023
@stream2000 stream2000 self-assigned this Jan 18, 2024
@stream2000 stream2000 marked this pull request as ready for review February 17, 2024 03:33
throw new IllegalArgumentException("No PartitionTTLStrategyType found for class name: " + className);
}

public static List<String> getNames() {
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to getPartitionTTLStrategyNames?

@@ -583,6 +594,12 @@ protected void runTableServicesInline(HoodieTable table, HoodieCommitMetadata me
metadata.addMetadata(HoodieClusteringConfig.SCHEDULE_INLINE_CLUSTERING.key(), "true");
inlineScheduleClustering(extraMetadata);
}

// Do an inline partition ttl management if enabled
if (config.isAutoPartitionTTL()) {
Copy link
Contributor

@leesf leesf Feb 22, 2024

Choose a reason for hiding this comment

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

isInlinePartitionTTLEnabled would be better?

@github-actions github-actions bot added the size:XL PR with lines of changes > 1000 label Feb 26, 2024
/**
* Strategy for ttl management.
*/
public interface TTLStrategy {
Copy link
Contributor

Choose a reason for hiding this comment

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

this interface is empty with no methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's reserved for row-level ttl management.

@hudi-bot
Copy link

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

Copy link
Contributor

@leesf leesf left a comment

Choose a reason for hiding this comment

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

Please also update the docs to show how to use the partition TTL.

@leesf leesf merged commit 89d267a into apache:master Feb 28, 2024
30 checks passed
* @param referenceTime last commit time or creation time for partition
*/
protected boolean isPartitionExpired(String referenceTime) {
String expiredTime = instantTimePlusMillis(fixInstantTimeCompatibility(referenceTime), ttlInMilis);
Copy link
Contributor

@xicm xicm Apr 25, 2024

Choose a reason for hiding this comment

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

Hi, fixInstantTimeCompatibility is called twice.

public static String instantTimePlusMillis(String timestamp, long milliseconds) {
try {
String timestampInMillis = fixInstantTimeCompatibility(timestamp);
LocalDateTime dt = LocalDateTime.parse(timestampInMillis, MILLIS_INSTANT_TIME_FORMATTER);
ZoneId zoneId = HoodieTimelineTimeZone.UTC.equals(commitTimeZone) ? ZoneId.of("UTC") : ZoneId.systemDefault();
return MILLIS_INSTANT_TIME_FORMATTER.format(dt.atZone(zoneId).toInstant().plusMillis(milliseconds).atZone(zoneId).toLocalDateTime());
} catch (DateTimeParseException e) {
throw new HoodieException(e);
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XL PR with lines of changes > 1000
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants