konstantinb commented on code in PR #8426: URL: https://github.com/apache/hadoop/pull/8426#discussion_r3455274073
########## hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ASharedThreadPoolEnabled.java: ########## @@ -0,0 +1,97 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.fs.s3a; + +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.hadoop.conf.Configuration; + +import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ASYNC_CLIENT_SHARED_THREADPOOL_ENABLED; +import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ASYNC_CLIENT_SHARED_THREADPOOL_SIZE; +import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_CLIENT_SHARED_THREADPOOL_ENABLED; +import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_CLIENT_SHARED_THREADPOOL_SIZE; + +/** + * Integration test demonstrating that the shared scheduled thread pool bounds + * thread growth across many S3AFileSystem instances. + * <p> + * Background (HADOOP-19862): each S3AFileSystem builds its own AWS SDK clients, + * and each client otherwise creates a 5-thread sdk-ScheduledExecutor pool, so + * creating many instances grows threads without bound. With the shared pool + * enabled, all clients of a type reuse a single pool, so the thread count stays + * bounded regardless of how many instances are created. + * <p> + * This test proves the fix direction (pool enabled, threads bounded); + * {@link ITestS3ASharedThreadPoolDisabled} is the control proving the opposite + * (pool disabled, threads grow). The two must be separate classes: the holders + * are private static final and memoize on first use, so the first configuration + * to reach them wins for the JVM. They stay isolated because hadoop-aws runs + * tests with reuseForks=false (a fresh JVM per test class); do not merge them + * into one class. + * <p> + * Exact thread counts depend on AWS SDK internals, so the assertion is on the + * bound (not exceeding the pool sizes), not an exact number. + */ +public class ITestS3ASharedThreadPoolEnabled extends AbstractS3ATestBase { Review Comment: @steveloughran thank you for your review and additional comments I went back and forth on this one. My current conclusion: teardown doesn't need to touch the pool, so I've left it out and documented the constraint — happy to add a hook if you'd prefer. On resources, it cleans up after itself: the threads are daemon, and it sets `allowCoreThreadTimeOut(true)` with a 60s keepalive, so idle threads time out and the pool is reaped at JVM exit — it won't accumulate or hold the JVM open. On isolation, a reset is the only lever — the holder is a JVM-global static that memoizes on first use and then ignores later config. Under `reuseForks=false` that reset is a no-op (each class already runs in its own fresh JVM), and under fork reuse it'd be worse: `shutdownNow()` on the shared pool would reject in-flight scheduled work for any other filesystem still using it. So the enabled/disabled cases are split into two classes, each in its own fork (which is what provides the isolation), and I've noted that in the javadocs. If you'd still like an explicit test-only shutdown hook regardless, I'd be happy to add one. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
