Copilot commented on code in PR #29008:
URL: https://github.com/apache/flink/pull/29008#discussion_r3850005639
##########
flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/history/HistoryServer.java:
##########
@@ -291,6 +291,8 @@ public HistoryServer(
config.get(HISTORY_SERVER_LAZY_FETCH_EXECUTOR_COMMON_POOL_SIZE);
int lazyFetchExecutorIndividualPoolSize =
config.get(HISTORY_SERVER_LAZY_FETCH_EXECUTOR_INDIVIDUAL_POOL_SIZE);
+ boolean retainRemoteBeyondLocalLimit =
+
config.get(HistoryServerOptions.HISTORY_SERVER_RETAIN_REMOTE_BEYOND_LOCAL_LIMIT);
Review Comment:
Because this boolean changes whether remote archives are deleted on refresh,
its effective value should be logged when the HistoryServer initializes.
Nothing currently records whether this retention behavior is enabled, leaving
operators unable to confirm the effective setting from the startup logs.
##########
flink-runtime-web/src/main/java/org/apache/flink/runtime/webmonitor/history/HistoryServerArchiveFetcher.java:
##########
@@ -240,9 +273,16 @@ void scanArchives(
&& processExpiredArchiveDeletion) {
events.addAll(cleanupExpiredArchives(archivesToRemove));
}
- // clean remote and local
if (!archivesBeyondRetainedLimit.isEmpty()) {
-
events.addAll(cleanupArchivesBeyondRetainedLimit(archivesBeyondRetainedLimit));
+ if (retainRemoteBeyondLocalLimit) {
+ // clean local only; the remote archive is left in place
and remains
+ // fetchable on demand (e.g. via LAZY archive load mode).
+ events.addAll(
+
cleanupLocalArchivesBeyondRetainedLimit(archivesBeyondRetainedLimit));
Review Comment:
This branch treats every rejection from the composite retention strategy as
a count-limit overflow, but that strategy also rejects archives whose TTL has
expired. With `historyserver.archive.retained-jobs=-1`, a configured
`historyserver.archive.retained-ttl`, and this flag enabled, TTL-expired
archives are therefore kept remotely, contradicting the option documentation
that the flag has no effect when the count limit is `-1` and bypassing the
existing remote TTL cleanup. Please limit the local-only path to archives
rejected by the count policy, or update the documented contract and tests if
retaining TTL-expired archives is intentional.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
