Skip to content

Commit d926cdd

Browse files
[#13800] feat(policy): Maintenance execution peers on Iceberg contents (#13885)
### What changes were proposed in this pull request? Split from #13801 (1/2). **Domain** execution peers for Epic #13764 / #13800. - `PolicyExecutionSpec` / `ExecutablePolicyContent` / `PolicyExecutionFields` - Four Iceberg maintenance contents carry peers at construction - TMS design notes on peers / `BuiltInType` identity **Follow-up (stack):** #13888 / #13887 — wire/DTO/SerDe + `minIntervalMs` registry. ### Why are the changes needed? TMS needs shared trigger / cooldown / option-bag concepts without freezing them onto every `PolicyContent`. Fix: #13800 ### Does this PR introduce _any_ user-facing change? API/domain surface for optional peers on Iceberg maintenance contents (REST wire lands in #13887). ### How was this patch tested? ```bash ./gradlew :api:test --tests 'org.apache.gravitino.policy.*' -PskipITs ``` --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 19d88d7 commit d926cdd

15 files changed

Lines changed: 1450 additions & 87 deletions

File tree

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.gravitino.policy;
20+
21+
import java.util.Objects;
22+
import javax.annotation.Nullable;
23+
24+
/**
25+
* Base for built-in Iceberg maintenance policy contents that carry optional execution peers.
26+
*
27+
* <p>Peers are supplied at construction as an immutable {@link PolicyExecutionSpec}. Subclasses
28+
* keep strategy fields separate and pass the spec through {@link
29+
* #AbstractExecutablePolicyContent(PolicyExecutionSpec)}. Wire and persistence JSON keep peer keys
30+
* flat; mapping to and from those keys is handled in {@code common} (custom Jackson serdes), not by
31+
* flattening the five fields onto this type.
32+
*/
33+
public abstract class AbstractExecutablePolicyContent implements ExecutablePolicyContent {
34+
35+
private final PolicyExecutionSpec executionSpec;
36+
37+
/**
38+
* Creates content with the given execution peers.
39+
*
40+
* @param executionSpec immutable peers; {@code null} is treated as {@link
41+
* PolicyExecutionSpec#empty()}
42+
*/
43+
protected AbstractExecutablePolicyContent(@Nullable PolicyExecutionSpec executionSpec) {
44+
this.executionSpec = executionSpec == null ? PolicyExecutionSpec.empty() : executionSpec;
45+
}
46+
47+
/** Creates content with unset execution peers. */
48+
protected AbstractExecutablePolicyContent() {
49+
this(PolicyExecutionSpec.empty());
50+
}
51+
52+
@Override
53+
public PolicyExecutionSpec executionSpec() {
54+
return executionSpec;
55+
}
56+
57+
/**
58+
* Validates universal {@link PolicyContent} rules plus shared execution-peer checks.
59+
*
60+
* <p>Does not enforce type-specific maintenance contracts (for example orphan cleanup forbidding
61+
* {@code commitTrigger}, or cron parsing). Those belong on concrete subclasses or the server
62+
* create/alter path with the real {@link Policy.BuiltInType}.
63+
*/
64+
@Override
65+
public void validate() throws IllegalArgumentException {
66+
ExecutablePolicyContent.super.validate();
67+
executionSpec().validate();
68+
}
69+
70+
/**
71+
* Whether execution fields match.
72+
*
73+
* @param that other content
74+
* @return true when execution fields are equal
75+
*/
76+
protected boolean executionFieldsEqual(AbstractExecutablePolicyContent that) {
77+
return Objects.equals(executionSpec(), that.executionSpec());
78+
}
79+
80+
/**
81+
* @return hash of execution fields
82+
*/
83+
protected int executionFieldsHashCode() {
84+
return Objects.hashCode(executionSpec());
85+
}
86+
87+
/**
88+
* Appends execution fields to a subclass {@code toString} body.
89+
*
90+
* <p>Option map <em>values</em> are never rendered (keys only) so accidental credentials are not
91+
* exposed through logs or exception messages.
92+
*
93+
* <p>Callers that already wrote subclass fields should append {@code ", "} before invoking this
94+
* method.
95+
*
96+
* @param sb builder already containing an opening brace and any subclass fields
97+
*/
98+
protected void appendExecutionFields(StringBuilder sb) {
99+
PolicyExecutionSpec spec = executionSpec();
100+
sb.append("timeTrigger='")
101+
.append(spec.timeTrigger())
102+
.append('\'')
103+
.append(", commitTrigger=")
104+
.append(spec.commitTrigger())
105+
.append(", minIntervalMs=")
106+
.append(spec.minIntervalMs())
107+
.append(", jobOptionsKeys=")
108+
.append(spec.jobOptions().keySet())
109+
.append(", updaterOptionsKeys=")
110+
.append(spec.updaterOptions().keySet());
111+
}
112+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.gravitino.policy;
20+
21+
/**
22+
* Policy content that carries immutable execution peers for automated maintenance.
23+
*
24+
* <p>Row-filter, column-mask, and custom contents are not executable in this sense. Callers that
25+
* need peers should use {@code instanceof ExecutablePolicyContent} (or a concrete maintenance
26+
* content type) and read {@link #executionSpec()} rather than assuming every {@link PolicyContent}
27+
* exposes triggers or option bags.
28+
*/
29+
public interface ExecutablePolicyContent extends PolicyContent {
30+
31+
/**
32+
* @return immutable execution peers; never {@code null}
33+
*/
34+
PolicyExecutionSpec executionSpec();
35+
}

‎api/src/main/java/org/apache/gravitino/policy/IcebergDataCompactionContent.java‎

Lines changed: 54 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
import org.apache.gravitino.MetadataObject;
3333

3434
/** Built-in policy content for Iceberg compaction strategy. */
35-
public class IcebergDataCompactionContent implements PolicyContent {
35+
public class IcebergDataCompactionContent extends AbstractExecutablePolicyContent {
3636
/** Property key for strategy type. */
3737
public static final String STRATEGY_TYPE_KEY = "strategy.type";
3838
/** Strategy type value for iceberg data compaction. */
@@ -130,7 +130,7 @@ public class IcebergDataCompactionContent implements PolicyContent {
130130

131131
/** Default constructor for Jackson deserialization only. */
132132
private IcebergDataCompactionContent() {
133-
this(null, null, null, null, null, null, null, null);
133+
this(null, null, null, null, null, null, null, null, PolicyExecutionSpec.empty());
134134
}
135135

136136
IcebergDataCompactionContent(
@@ -142,6 +142,29 @@ private IcebergDataCompactionContent() {
142142
String rewriteStrategy,
143143
String sortOrder,
144144
Map<String, String> rewriteOptions) {
145+
this(
146+
minDataFileMse,
147+
minDeleteFileNumber,
148+
dataFileMseWeight,
149+
deleteFileNumberWeight,
150+
maxPartitionNum,
151+
rewriteStrategy,
152+
sortOrder,
153+
rewriteOptions,
154+
PolicyExecutionSpec.empty());
155+
}
156+
157+
IcebergDataCompactionContent(
158+
Long minDataFileMse,
159+
Long minDeleteFileNumber,
160+
Long dataFileMseWeight,
161+
Long deleteFileNumberWeight,
162+
Long maxPartitionNum,
163+
String rewriteStrategy,
164+
String sortOrder,
165+
Map<String, String> rewriteOptions,
166+
PolicyExecutionSpec executionSpec) {
167+
super(executionSpec);
145168
// Nullable inputs are treated as "use default" to simplify policy creation.
146169
this.minDataFileMse = minDataFileMse == null ? DEFAULT_MIN_DATA_FILE_MSE : minDataFileMse;
147170
this.minDeleteFileNumber =
@@ -267,7 +290,7 @@ public Map<String, Object> rules() {
267290

268291
@Override
269292
public void validate() throws IllegalArgumentException {
270-
PolicyContent.super.validate();
293+
super.validate();
271294
// All fields are defaulted in the constructor, so only range checks are needed here.
272295
Preconditions.checkArgument(minDataFileMse >= 0, "minDataFileMse must be >= 0");
273296
Preconditions.checkArgument(minDeleteFileNumber >= 0, "minDeleteFileNumber must be >= 0");
@@ -314,7 +337,8 @@ public boolean equals(Object o) {
314337
return false;
315338
}
316339
IcebergDataCompactionContent that = (IcebergDataCompactionContent) o;
317-
return Objects.equals(minDataFileMse, that.minDataFileMse)
340+
return executionFieldsEqual(that)
341+
&& Objects.equals(minDataFileMse, that.minDataFileMse)
318342
&& Objects.equals(minDeleteFileNumber, that.minDeleteFileNumber)
319343
&& Objects.equals(dataFileMseWeight, that.dataFileMseWeight)
320344
&& Objects.equals(deleteFileNumberWeight, that.deleteFileNumberWeight)
@@ -327,6 +351,7 @@ public boolean equals(Object o) {
327351
@Override
328352
public int hashCode() {
329353
return Objects.hash(
354+
executionFieldsHashCode(),
330355
minDataFileMse,
331356
minDeleteFileNumber,
332357
dataFileMseWeight,
@@ -339,25 +364,30 @@ public int hashCode() {
339364

340365
@Override
341366
public String toString() {
342-
return "IcebergDataCompactionContent{"
343-
+ "minDataFileMse="
344-
+ minDataFileMse
345-
+ ", minDeleteFileNumber="
346-
+ minDeleteFileNumber
347-
+ ", dataFileMseWeight="
348-
+ dataFileMseWeight
349-
+ ", deleteFileNumberWeight="
350-
+ deleteFileNumberWeight
351-
+ ", maxPartitionNum="
352-
+ maxPartitionNum
353-
+ ", rewriteStrategy='"
354-
+ rewriteStrategy
355-
+ '\''
356-
+ ", sortOrder='"
357-
+ sortOrder
358-
+ '\''
359-
+ ", rewriteOptions="
360-
+ rewriteOptions
361-
+ '}';
367+
StringBuilder sb =
368+
new StringBuilder("IcebergDataCompactionContent{")
369+
.append("minDataFileMse=")
370+
.append(minDataFileMse)
371+
.append(", minDeleteFileNumber=")
372+
.append(minDeleteFileNumber)
373+
.append(", dataFileMseWeight=")
374+
.append(dataFileMseWeight)
375+
.append(", deleteFileNumberWeight=")
376+
.append(deleteFileNumberWeight)
377+
.append(", maxPartitionNum=")
378+
.append(maxPartitionNum)
379+
.append(", rewriteStrategy='")
380+
.append(rewriteStrategy)
381+
.append('\'')
382+
.append(", sortOrder='")
383+
.append(sortOrder)
384+
.append('\'')
385+
// Never render rewriteOptions values; same redaction as jobOptions / updaterOptions.
386+
.append(", rewriteOptionsKeys=")
387+
.append(rewriteOptions.keySet())
388+
.append(", ");
389+
appendExecutionFields(sb);
390+
sb.append('}');
391+
return sb.toString();
362392
}
363393
}

‎api/src/main/java/org/apache/gravitino/policy/IcebergOrphanFileRemovalContent.java‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
import org.apache.gravitino.MetadataObject;
3232

3333
/** Configuration for table-level Iceberg orphan cleanup on an optimizer invocation. */
34-
public class IcebergOrphanFileRemovalContent implements PolicyContent {
34+
public class IcebergOrphanFileRemovalContent extends AbstractExecutablePolicyContent {
3535
/** Property key for strategy type. */
3636
public static final String STRATEGY_TYPE_KEY = "strategy.type";
3737
/** Property key for the job template. */
@@ -64,10 +64,19 @@ public class IcebergOrphanFileRemovalContent implements PolicyContent {
6464
private final boolean dryRun;
6565

6666
private IcebergOrphanFileRemovalContent() {
67-
this(DEFAULT_OLDER_THAN_DAYS, null, DEFAULT_DRY_RUN);
67+
this(DEFAULT_OLDER_THAN_DAYS, null, DEFAULT_DRY_RUN, PolicyExecutionSpec.empty());
6868
}
6969

7070
IcebergOrphanFileRemovalContent(long olderThanDays, @Nullable String location, boolean dryRun) {
71+
this(olderThanDays, location, dryRun, PolicyExecutionSpec.empty());
72+
}
73+
74+
IcebergOrphanFileRemovalContent(
75+
long olderThanDays,
76+
@Nullable String location,
77+
boolean dryRun,
78+
@Nullable PolicyExecutionSpec executionSpec) {
79+
super(executionSpec);
7180
this.olderThanDays = olderThanDays;
7281
this.location = location;
7382
this.dryRun = dryRun;
@@ -136,7 +145,10 @@ public static void validateOlderThanDays(long days) {
136145

137146
@Override
138147
public void validate() {
139-
PolicyContent.super.validate();
148+
super.validate();
149+
Preconditions.checkArgument(
150+
!Boolean.TRUE.equals(executionSpec().commitTrigger()),
151+
"commitTrigger is not allowed for orphan file removal");
140152
validateOlderThanDays(olderThanDays);
141153
Preconditions.checkArgument(
142154
location == null || StringUtils.isNotBlank(location), "location must not be blank");
@@ -151,13 +163,14 @@ public boolean equals(Object other) {
151163
return false;
152164
}
153165
IcebergOrphanFileRemovalContent that = (IcebergOrphanFileRemovalContent) other;
154-
return olderThanDays == that.olderThanDays
166+
return executionFieldsEqual(that)
167+
&& olderThanDays == that.olderThanDays
155168
&& dryRun == that.dryRun
156169
&& Objects.equals(location, that.location);
157170
}
158171

159172
@Override
160173
public int hashCode() {
161-
return Objects.hash(olderThanDays, location, dryRun);
174+
return Objects.hash(executionFieldsHashCode(), olderThanDays, location, dryRun);
162175
}
163176
}

‎api/src/main/java/org/apache/gravitino/policy/IcebergRewriteManifestsContent.java‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,16 @@
2424
import java.util.LinkedHashMap;
2525
import java.util.Map;
2626
import java.util.Set;
27+
import javax.annotation.Nullable;
2728
import org.apache.gravitino.MetadataObject;
2829

2930
/**
3031
* Placeholder built-in policy content for Iceberg manifest rewrite.
3132
*
3233
* <p>Configurable thresholds will be added when the rewrite-manifests policy lands; this class
33-
* registers the built-in type for TMS task mapping.
34+
* registers {@link Policy.BuiltInType#ICEBERG_REWRITE_MANIFESTS}.
3435
*/
35-
public class IcebergRewriteManifestsContent implements PolicyContent {
36+
public class IcebergRewriteManifestsContent extends AbstractExecutablePolicyContent {
3637
/** Property key for strategy type. */
3738
public static final String STRATEGY_TYPE_KEY = "strategy.type";
3839
/** Property key for the job template. */
@@ -51,7 +52,13 @@ public class IcebergRewriteManifestsContent implements PolicyContent {
5152
MetadataObject.Type.CATALOG, MetadataObject.Type.SCHEMA, MetadataObject.Type.TABLE);
5253

5354
/** Default constructor for Jackson deserialization only. */
54-
public IcebergRewriteManifestsContent() {}
55+
public IcebergRewriteManifestsContent() {
56+
this(PolicyExecutionSpec.empty());
57+
}
58+
59+
IcebergRewriteManifestsContent(@Nullable PolicyExecutionSpec executionSpec) {
60+
super(executionSpec);
61+
}
5562

5663
@Override
5764
public Set<MetadataObject.Type> supportedObjectTypes() {
@@ -75,16 +82,20 @@ public Map<String, Object> rules() {
7582

7683
@Override
7784
public boolean equals(Object other) {
78-
return other instanceof IcebergRewriteManifestsContent;
85+
return other instanceof IcebergRewriteManifestsContent
86+
&& executionFieldsEqual((IcebergRewriteManifestsContent) other);
7987
}
8088

8189
@Override
8290
public int hashCode() {
83-
return IcebergRewriteManifestsContent.class.hashCode();
91+
return executionFieldsHashCode();
8492
}
8593

8694
@Override
8795
public String toString() {
88-
return "IcebergRewriteManifestsContent{}";
96+
StringBuilder sb = new StringBuilder("IcebergRewriteManifestsContent{");
97+
appendExecutionFields(sb);
98+
sb.append('}');
99+
return sb.toString();
89100
}
90101
}

0 commit comments

Comments
 (0)