[SPARK-58468][CORE] Support on-demand loading for single event logs in SHS - #57676
[SPARK-58468][CORE] Support on-demand loading for single event logs in SHS#57676pan3793 wants to merge 6 commits into
Conversation
Assisted-by: gpt-5.6-terra
Assisted-by: gpt-5.6-terra
|
cc @dongjoon-hyun @sarutak, could you please take a look? thank you in advance |
| "a fine:mind$dollar{bills}.1", None, Some(CompressionCodec.LZ4))) | ||
| } | ||
|
|
||
| test("Event log file names") { |
There was a problem hiding this comment.
Could we add a test case for getLogFileNames("app1", None) as well? It would cover the path
where no attempt ID suffix is appended.
| } | ||
| } | ||
|
|
||
| test("Support spark.history.fs.eventLog.single.onDemandLoadEnabled") { |
There was a problem hiding this comment.
The tests properly isolate single vs rolling by disabling the other. Would it be worth adding one test with both enabled (the default state) to verify the combined fallback behavior?
Also, the existing rolling on-demand test checks provider.getAppUI("nonexist", None).isEmpty to ensure no phantom entries are left. Could we add the same assertion to the single on-demand test?
There was a problem hiding this comment.
Added combined coverage with both configurations explicitly enabled, plus the missing-app assertion, in 0311765.
Assisted-by: gpt-5.6-terra
| .createWithDefault(true) | ||
|
|
||
| val EVENT_LOG_SINGLE_ON_DEMAND_LOAD_ENABLED = | ||
| ConfigBuilder("spark.history.fs.eventLog.single.onDemandLoadEnabled") |
There was a problem hiding this comment.
Do you have more items for this new namespace, spark.history.fs.eventLog.single? If not, shall we avoid to introduce a new namespace, @pan3793 ?
There was a problem hiding this comment.
@dongjoon-hyun thanks for checking, I removed the single part from the config key, also updated PR description and UT.
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM (except one namespace issue).
…cala Co-authored-by: Dongjoon Hyun <dongjoon@apache.org>
Co-authored-by: Dongjoon Hyun <dongjoon@apache.org>
dongjoon-hyun
left a comment
There was a problem hiding this comment.
Thank you for updating, @pan3793 .
…n SHS ### What changes were proposed in this pull request? This PR extends History Server on-demand event-log loading to single event logs. It adds `spark.history.fs.eventLog.onDemandLoadEnabled`, which is independent from the existing rolling event-log configuration. When enabled, the History Server probes completed, compressed, and in-progress single event-log names after a requested application is absent from the listing. ### Why are the changes needed? Single event logs remain supported, but on-demand loading currently only supports rolling event logs. A requested application with a single event log can therefore return Not Found until periodic scanning discovers its log. ### Does this PR introduce _any_ user-facing change? Yes. History Server can load single event logs on demand. The new `spark.history.fs.eventLog.onDemandLoadEnabled` configuration defaults to `true`. ### How was this patch tested? Added tests for: - enabled and disabled single event-log on-demand loading - uncompressed and LZF-compressed logs - in-progress logs and cleaner safety - attempt IDs - multiple log directories - scan-disabled directories Ran the focused `FsHistoryProviderSuite` and `SingleEventLogFileWriterSuite` tests. Also ran `SparkConfigBindingPolicySuite`. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: gpt-5.6-terra Closes #57676 from pan3793/shs-on-demand-v1-log-loading. Authored-by: Cheng Pan <pan3793@gmail.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 3b581e0) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
What changes were proposed in this pull request?
This PR extends History Server on-demand event-log loading to single event logs.
It adds
spark.history.fs.eventLog.onDemandLoadEnabled, which is independent from the existing rolling event-log configuration. When enabled, the History Server probes completed, compressed, and in-progress single event-log names after a requested application is absent from the listing.Why are the changes needed?
Single event logs remain supported, but on-demand loading currently only supports rolling event logs. A requested application with a single event log can therefore return Not Found until periodic scanning discovers its log.
Does this PR introduce any user-facing change?
Yes. History Server can load single event logs on demand. The new
spark.history.fs.eventLog.onDemandLoadEnabledconfiguration defaults totrue.How was this patch tested?
Added tests for:
Ran the focused
FsHistoryProviderSuiteandSingleEventLogFileWriterSuitetests. Also ranSparkConfigBindingPolicySuite.Was this patch authored or co-authored using generative AI tooling?
Generated-by: gpt-5.6-terra