[
https://issues.apache.org/jira/browse/HADOOP-19862?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18090006#comment-18090006
]
ASF GitHub Bot commented on HADOOP-19862:
-----------------------------------------
konstantinb commented on PR #8426:
URL: https://github.com/apache/hadoop/pull/8426#issuecomment-4745744957
Thanks again for the thorough review, @ajfabbri — much appreciated.
> I have two requested changes to (1) add to `core-default.xml` (2) remove
superfluous `volatile`.
Both done — removed the `volatile` (the field is only accessed inside the
`synchronized` method) and added the shared-thread-pool keys to
`core-default.xml` with their defaults. Also addressed the two inline nits:
`@VisibleForTesting` on `createScheduledExecutor`, and explicit size/keepalive
value assertions in `TestSharedScheduledExecutor`.
> Final thought / question: It would be cool to add a test which actually
causes runaway thread growth with S3A FS instances, and compares it w/ and w/o
shared pools.
I had the same instinct, so it's in this PR rather than a follow-up. Two
ITests:
- `ITestS3ASharedThreadPoolDisabled` — creates 20 uncached `S3AFileSystem`
instances, abandons them, then forces GC until a `WeakReference` to one clears
(proving the instance was really collected, since `System.gc()` is only a
hint). Without sharing, the per-client `sdk-ScheduledExecutor` pools survive
collection and the thread count far exceeds a single pool.
- `ITestS3ASharedThreadPoolEnabled` — same scenario with the shared pool on;
the thread count stays bounded.
Together they demonstrate the impact of
[aws-sdk-java-v2#1690](https://github.com/aws/aws-sdk-java-v2/issues/1690) —
the "leaked when the client is evicted" thread growth without the shared pool,
and bounded with it.
> Thinking out loud on the lifecycle and ownership question... The fact that
these are opt-in configs that default to disabled mitigates a lot of the risk...
Agreed. The opt-in, default-disabled design was deliberate, for exactly the
risk you mention around qualifying different workloads — the change has no
effect on existing deployments until someone explicitly enables it. The shared
executor also uses `allowCoreThreadTimeOut(true)` with a 60s keepalive, so idle
threads time out and the pool shrinks back to near zero when there's no work,
instead of holding threads for the life of the JVM. As an extra check that
turning it on is safe, I ran the full hadoop-aws integration suite (against
LocalStack) both with all four pools enabled and with the default; the
pass/fail set was identical except the disabled-control test above, which by
design expects the leak — so enabling the pools didn't perturb the rest of the
suite.
> Another thought for potential future work: it would be nice if the pool
sizes were auto-tuned.
Strongly agree on the "too many knobs" point. The current defaults are at
least a reasonable baseline — size 5 mirrors the SDK's own per-client default
(`Executors.newScheduledThreadPool(5)`), and the 60s keepalive adds the
idle-thread timeout the SDK's default executor doesn't have. If we revisit
sizing, I'd lean away from a control loop and toward the self-trimming we
already have: since idle threads already time out, over-sizing is nearly free
while under-sizing only costs scheduling latency, so a generous cap (e.g.
derived from `availableProcessors()`) that the keepalive trims back to the
actual working set removes most of the need to pick an exact number — and since
this pool just fires small retry/timeout/refresh tasks, even 5 is rarely the
bottleneck. The higher-value first step is probably observability rather than
tuning: surfacing live scheduler-thread count and scheduling latency (e.g. via
`IOStatistics`) so it's visible whether a pool is ever saturated before anyone
reaches for a knob.
> S3A: Thread leak from AWS SDK v2 ScheduledExecutorService
> ---------------------------------------------------------
>
> Key: HADOOP-19862
> URL: https://issues.apache.org/jira/browse/HADOOP-19862
> Project: Hadoop Common
> Issue Type: Bug
> Reporter: Konstantin Bereznyakov
> Priority: Major
> Labels: pull-request-available
>
> AWS SDK v2 S3 clients create internal ScheduledExecutorService instances that
> accumulate over time, causing unbounded thread growth. Thread dumps show
> thousands of sdk-ScheduledExecutor-* threads in processes that create
> multiple S3AFileSystem instances.
> Environment
> - Hadoop 3.4.x with AWS SDK v2
> - Not observed in Hadoop 3.3.x (AWS SDK v1)
> Observed Behavior
> Thread dump comparison:
> Hadoop 3.3.x (AWS SDK v1): Normal thread count
> Hadoop 3.4.x (AWS SDK v2): 1600+ "sdk-ScheduledExecutor-*" threads
> Thread pattern:
> "sdk-ScheduledExecutor-0-0" daemon prio=5 waiting
> "sdk-ScheduledExecutor-0-1" daemon prio=5 waiting
> ...
> "sdk-ScheduledExecutor-0-4" daemon prio=5 waiting
> "sdk-ScheduledExecutor-1-0" daemon prio=5 waiting
> ...
> Root Cause
> AWS SDK v2's SdkDefaultClientBuilder creates a ScheduledThreadPoolExecutor
> with 5 threads per client when no executor is explicitly provided
> (https://github.com/aws/aws-sdk-java-v2/issues/1690):
> Executors.newScheduledThreadPool(5,
> new
> ThreadFactoryBuilder().threadNamePrefix("sdk-ScheduledExecutor").build())
> These threads are used for retry scheduling, timeout handling, and
> credential refresh.
> Contributing Factors
> 1. AbstractFileSystem has no caching
> Unlike FileSystem.get() which uses CACHE.get(uri, conf),
> AbstractFileSystem.get() always creates new instances:
> // AbstractFileSystem.java:263-266
> public static AbstractFileSystem get(final URI uri, final Configuration
> conf) {
> return createFileSystem(uri, conf); // NO CACHING
> }
> Each FileContext.getFileContext() call with an S3 URI creates:
> - New AbstractFileSystem (S3A)
> - New S3AFileSystem
> - New S3Client
> - 5 new sdk-ScheduledExecutor threads
> 2. S3Client threads not released on close
> As documented in https://github.com/aws/aws-sdk-java-v2/issues/1690:
> "When using cached, ephemeral clients, I can see that the scheduled thread
> pool will at times be leaked when the aws client is evicted"
> 3. Multiple client types affected
> S3A creates multiple AWS SDK clients:
> - S3Client (sync)
> - S3AsyncClient
> - STS client (for delegation tokens)
> - KMS client (for encryption)
> Each client instance creates its own 5-thread pool.
> Impact
> - Unbounded thread growth in any process using S3A
> - Resource exhaustion leading to OOM or system instability
> - Particularly affects YARN NodeManager, Spark drivers/executors, and other
> services that create many filesystem instances
> Related
> - https://github.com/aws/aws-sdk-java-v2/issues/1690 - SDK issue
> documenting the problem
> - https://github.com/aws/aws-sdk-java-v2/pull/4002 - SDK fix allowing
> shared executor configuration
> - HADOOP-19624 - Similar thread leak in ABFS (AbfsClientThrottlingAnalyzer)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
