mynameborat commented on code in PR #26314:
URL: https://github.com/apache/beam/pull/26314#discussion_r1170536592
##########
runners/samza/src/main/java/org/apache/beam/runners/samza/translation/ConfigBuilder.java:
##########
@@ -131,6 +117,30 @@ public Config build() {
}
}
+ @VisibleForTesting
+ static Map<String, String> createBundleConfig(
+ SamzaPipelineOptions options, Map<String, String> config) {
+ ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
+ builder.put(MAX_CONCURRENCY, String.valueOf(options.getMaxBundleSize()));
+
+ if (options.getMaxBundleSize() > 1) {
+ final int threadPoolSize =
ConfigUtils.asJobConfig(config).getThreadPoolSize();
+ LOG.info("Remove threadPoolSize configs when maxBundleSize > 1");
+ builder.put(JOB_CONTAINER_THREAD_POOL_SIZE, "0");
+ builder.put(JOB_AUTOSIZING_CONTAINER_THREAD_POOL_SIZE, "0");
+
+ if (threadPoolSize > 1 && options.getNumThreadsForProcessElement() <= 1)
{
+ // In case the user sets the thread pool through samza config instead
options,
+ // set the bundle thread pool size based on container thread pool
config
+ // this allows Samza auto-sizing to tune the threads
+ LOG.info("Convert threadPoolSize {} to numThreadsForProcessElement",
threadPoolSize);
+ // NumThreadsForProcessElement in option is the source of truth
+ options.setNumThreadsForProcessElement(threadPoolSize);
Review Comment:
I feel the inconsistency in behavior is as well confusing to the users. If
we want to establish user code as the highest precedence, maybe we should not
tie autosizing `job.autosizing.container.thread.pool.size` into this.
Consider this example
1. user configures `job.container.thread.pool.size`, it is reflected in the
application behavior
2. autosizing is enabled and user configuration no longer is reflected in
the application behavior
3. User sets it in code and the change is reflected in application behavior
user configuration << framework configuration override << user code
Also consider autosizing devs/support team not realizing knobs x, y and z
follows auto-sizer values as higher precedence on their end vs a, b follows
user values as higher precedence.
>The other problem is that we need a way for user to override without
turning off autosizing
I think the recommended way for now is to use auto-sizer tool to overrides
specific knobs.
--
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]
