Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PLANNER-1553] Able to configure thread pool queue size in execution … #1862

Merged
merged 1 commit into from Jul 11, 2019

Conversation

tkobayas
Copy link
Contributor

…server

It doesn't contain a unit test because I'm not sure about the way/place for such a test (But I tested manually on my RHPAM 7.3.1 and confirmed it works expectedly). If there is a suitable way/place, please kindly guide me. Thanks!

@@ -88,12 +88,16 @@ public void init(KieServerImpl kieServer, KieServerRegistry registry) {
// If new jobs are submitted and all threads are busy, the default reject policy will kick in.
int availableProcessorCount = Runtime.getRuntime().availableProcessors();
int resolvedActiveThreadCount = Math.max(1, availableProcessorCount - 2);
int queueSize = Integer.parseInt(System.getProperty(
KieServerConstants.KIE_OPTAPLANNER_THREAD_POOL_QUEUE_SIZE, String.valueOf(resolvedActiveThreadCount)));
logger.info("Creating a ThreadPoolExecutor with corePoolSize = " + resolvedActiveThreadCount + ","
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this information important enough to deserve info log level? Wouldn't be enough to use debug level?

Copy link
Contributor Author

@tkobayas tkobayas Jul 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under impression that component's configuration values deserve INFO level. And also helpful from support point of view.

For example)

2019-07-10 16:26:36,400 INFO  [org.jbpm.executor.impl.ExecutorImpl] (EJB default - 1) Starting jBPM Executor Component ...
         - Thread Pool Size: 1
         - Retries per Request: 3
         - Load from storage interval: 0 SECONDS (if less or equal 0 only initial sync with storage)

But if you push again for DEBUG level, it's fine for me to change. Please let me know your thought.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable, lets keep there INFO.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sutaakar ok, I 'll presume you're ok with merging this now?

@@ -112,6 +112,8 @@
public static final String KIE_SERVER_ROUTER = "org.kie.server.router";
public static final String KIE_SERVER_ROUTER_ATTEMPT_INTERVAL = "org.kie.server.router.connect";

public static final String KIE_OPTAPLANNER_THREAD_POOL_QUEUE_SIZE = "org.optaplanner.server.ext.thread.pool.queue.size";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the ".ext" in the name? Which other properties do that convention too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property is used only for OptaplannerKieServerExtension and it's identified by "org.optaplanner.server.ext.XXX".

https://github.com/kiegroup/droolsjbpm-integration/blob/7.23.x/kie-server-parent/kie-server-api/src/main/java/org/kie/server/api/KieServerConstants.java#L31

However, if you suggest any property name, I will follow :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this name makes sense. Thanks!

@ge0ffrey ge0ffrey merged commit 8b82336 into kiegroup:master Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants