-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[flink] Support table batching for orphan file cleanup #9247
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
base: master
Are you sure you want to change the base?
Changes from all commits
2639012
0a27a8f
8022bb0
026cf05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,9 +70,11 @@ | |
| import static org.apache.flink.util.Preconditions.checkState; | ||
| import static org.apache.paimon.utils.Preconditions.checkArgument; | ||
|
|
||
| /** Flink {@link OrphanFilesClean}, it will submit a job for a table. */ | ||
| /** Flink {@link OrphanFilesClean}, it will submit jobs in table batches. */ | ||
| public class FlinkOrphanFilesClean extends OrphanFilesClean { | ||
|
|
||
| public static final int DEFAULT_TABLE_BATCH_SIZE = 10; | ||
|
|
||
| protected static final Logger LOG = LoggerFactory.getLogger(FlinkOrphanFilesClean.class); | ||
|
|
||
| @Nullable protected final Integer parallelism; | ||
|
|
@@ -436,49 +438,98 @@ public static CleanOrphanFilesResult executeDatabaseOrphanFiles( | |
| String databaseName, | ||
| @Nullable String tableName) | ||
| throws Catalog.DatabaseNotExistException, Catalog.TableNotExistException { | ||
| return executeDatabaseOrphanFiles( | ||
| env, | ||
| catalog, | ||
| olderThanMillis, | ||
| dryRun, | ||
| parallelism, | ||
| databaseName, | ||
| tableName, | ||
| DEFAULT_TABLE_BATCH_SIZE); | ||
| } | ||
|
|
||
| public static CleanOrphanFilesResult executeDatabaseOrphanFiles( | ||
| StreamExecutionEnvironment env, | ||
| Catalog catalog, | ||
| long olderThanMillis, | ||
| boolean dryRun, | ||
| @Nullable Integer parallelism, | ||
| String databaseName, | ||
| @Nullable String tableName, | ||
| @Nullable Integer tableBatchSize) | ||
| throws Catalog.DatabaseNotExistException, Catalog.TableNotExistException { | ||
| List<String> tableNames = Collections.singletonList(tableName); | ||
| if (tableName == null || "*".equals(tableName)) { | ||
| tableNames = catalog.listTables(databaseName); | ||
| } | ||
|
|
||
| List<DataStream<CleanOrphanFilesResult>> orphanFilesCleans = | ||
| new ArrayList<>(tableNames.size()); | ||
| for (String t : tableNames) { | ||
| Identifier identifier = new Identifier(databaseName, t); | ||
| Table table = catalog.getTable(identifier); | ||
| checkArgument( | ||
| table instanceof FileStoreTable, | ||
| "Only FileStoreTable supports remove-orphan-files action. The table type is '%s'.", | ||
| table.getClass().getName()); | ||
|
|
||
| DataStream<CleanOrphanFilesResult> clean = | ||
| new FlinkOrphanFilesClean( | ||
| (FileStoreTable) table, olderThanMillis, dryRun, parallelism) | ||
| .doOrphanClean(env); | ||
| if (clean != null) { | ||
| orphanFilesCleans.add(clean); | ||
| int batchSize = tableBatchSize == null ? DEFAULT_TABLE_BATCH_SIZE : tableBatchSize; | ||
| checkArgument(batchSize > 0, "Table batch size must be greater than 0."); | ||
|
|
||
| long deletedFilesCount = 0; | ||
| long deletedFilesLenInBytes = 0; | ||
| for (int start = 0; start < tableNames.size(); start += batchSize) { | ||
| int end = Math.min(start + batchSize, tableNames.size()); | ||
| int batchNumber = start / batchSize + 1; | ||
| int tableCount = end - start; | ||
| long batchStart = System.currentTimeMillis(); | ||
| LOG.info( | ||
| "Starting orphan files clean batch #{} with {} tables.", | ||
| batchNumber, | ||
| tableCount); | ||
|
|
||
| List<DataStream<CleanOrphanFilesResult>> orphanFilesCleans = | ||
| new ArrayList<>(tableCount); | ||
| for (String t : tableNames.subList(start, end)) { | ||
| Identifier identifier = new Identifier(databaseName, t); | ||
| Table table = catalog.getTable(identifier); | ||
| checkArgument( | ||
| table instanceof FileStoreTable, | ||
| "Only FileStoreTable supports remove-orphan-files action. The table type is '%s'.", | ||
| table.getClass().getName()); | ||
|
|
||
| DataStream<CleanOrphanFilesResult> clean = | ||
| new FlinkOrphanFilesClean( | ||
| (FileStoreTable) table, | ||
| olderThanMillis, | ||
| dryRun, | ||
| parallelism) | ||
| .doOrphanClean(env); | ||
| if (clean != null) { | ||
| orphanFilesCleans.add(clean); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| DataStream<CleanOrphanFilesResult> result = null; | ||
| for (DataStream<CleanOrphanFilesResult> clean : orphanFilesCleans) { | ||
| if (result == null) { | ||
| result = clean; | ||
| } else { | ||
| result = result.union(clean); | ||
| DataStream<CleanOrphanFilesResult> result = null; | ||
| for (DataStream<CleanOrphanFilesResult> clean : orphanFilesCleans) { | ||
| result = result == null ? clean : result.union(clean); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't quite understand the execution process here—is it concurrent execution? Why is it described in terms of batches?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, tables within one batch are added to the same Flink JobGraph through Different batches are executed sequentially. Therefore, the parameter limits the number of tables included in each submitted Flink job, rather than directly limiting task concurrency. Does this execution model match what you had in mind, or would you prefer a different way to control resource usage? |
||
| } | ||
|
|
||
| CleanOrphanFilesResult batchResult = executeAndAggregateResults(result, batchNumber); | ||
| deletedFilesCount += batchResult.getDeletedFileCount(); | ||
| deletedFilesLenInBytes += batchResult.getDeletedFileTotalLenInBytes(); | ||
| LOG.info( | ||
| "Finished orphan files clean batch #{} with {} tables in {} ms.", | ||
| batchNumber, | ||
| tableCount, | ||
| System.currentTimeMillis() - batchStart); | ||
| } | ||
|
|
||
| return sum(result); | ||
| return new CleanOrphanFilesResult(deletedFilesCount, deletedFilesLenInBytes); | ||
| } | ||
|
|
||
| private static CleanOrphanFilesResult sum(DataStream<CleanOrphanFilesResult> deleted) { | ||
| private static CleanOrphanFilesResult executeAndAggregateResults( | ||
| DataStream<CleanOrphanFilesResult> cleanResults, int batchNumber) { | ||
| long deletedFilesCount = 0; | ||
| long deletedFilesLenInBytes = 0; | ||
| if (deleted != null) { | ||
| if (cleanResults != null) { | ||
| try { | ||
| CloseableIterator<CleanOrphanFilesResult> iterator = | ||
| deleted.global().executeAndCollect("OrphanFilesClean"); | ||
| cleanResults | ||
| .global() | ||
| .executeAndCollect( | ||
| String.format("OrphanFilesClean-Batch-%d", batchNumber)); | ||
| while (iterator.hasNext()) { | ||
| CleanOrphanFilesResult cleanOrphanFilesResult = iterator.next(); | ||
| deletedFilesCount += cleanOrphanFilesResult.getDeletedFileCount(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe max_table_number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This parameter controls the number of tables included in each batch (each submitted Flink job), rather than limiting the total number of tables to clean.
For example, 23 tables with a value of 10 will be processed in three batches: 10, 10, and 3. All 23 tables will still be cleaned.
max_table_numbermight be interpreted as a limit on the total number of tables to process. I thinktable_batch_sizebetter reflects the current semantics. Wouldmax_tables_per_batchbe clearer?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @JingsongLi, could you elaborate a bit on what you have in mind with
max_table_number?The current design submits all matched tables in sequential Flink jobs, and this parameter controls how many tables are included in each job.
I'm not sure whether your suggestion is only about the parameter name, or whether you have different semantics or an alternative design in mind. I'd like to understand your intention before changing the API.