You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "royteeuwen (via GitHub)" <gi...@apache.org> on 2024/07/19 07:27:04 UTC
[PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
royteeuwen opened a new pull request, #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38
(no comment)
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "royteeuwen (via GitHub)" <gi...@apache.org>.
royteeuwen commented on PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#issuecomment-2273282341
@kwin my PR didn't build because of JDK 21, but only upgrading Mockito to the latest version didn't fix that (https://github.com/apache/sling-org-apache-sling-event/pull/36).
Can you verify if this PR is ok and your pr can be closed?
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#issuecomment-2441812093
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-event&pullRequest=38) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-event&pullRequest=38&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-event&pullRequest=38&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-event&pullRequest=38&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [82.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-event&pullRequest=38&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-event&pullRequest=38&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-event&pullRequest=38)
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#issuecomment-2821849150
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-event&pullRequest=38) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-event&pullRequest=38&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-event&pullRequest=38&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-event&pullRequest=38&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [82.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-event&pullRequest=38&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-event&pullRequest=38&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-event&pullRequest=38)
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#issuecomment-2891601121
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-event&pullRequest=38) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-event&pullRequest=38&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-event&pullRequest=38&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-event&pullRequest=38&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [82.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-event&pullRequest=38&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-event&pullRequest=38&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-event&pullRequest=38)
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#discussion_r1769640388
##########
src/main/java/org/apache/sling/event/impl/EventingThreadPool.java:
##########
@@ -53,11 +55,19 @@ public class EventingThreadPool implements ThreadPool {
public @interface Config {
@AttributeDefinition(name = "Pool Size",
- description="The size of the thread pool. This pool is used to execute jobs and therefore "
- + "limits the maximum number of jobs executed in parallel.")
- int minPoolSize() default 35;
+ description = "The size of the thread pool. This pool is used to execute jobs and therefore "
+ + "limits the maximum number of jobs executed in parallel. "
+ + "A value of -1 is substituted with the number of available processors. "
+ + "A decimal number between 0.0 and 1.0 is treated as a fraction of available processors. "
+ + "For example 0.5 means half of the available processors. ")
+ double minPoolSize() default 35;
Review Comment:
I think that this has the chance to break a system where this value is defined as int. Have you checked how a running works if you provide an int? Is it automatically converted to a double?
##########
src/main/java/org/apache/sling/event/impl/EventingThreadPool.java:
##########
@@ -53,11 +55,19 @@ public class EventingThreadPool implements ThreadPool {
public @interface Config {
@AttributeDefinition(name = "Pool Size",
- description="The size of the thread pool. This pool is used to execute jobs and therefore "
- + "limits the maximum number of jobs executed in parallel.")
- int minPoolSize() default 35;
+ description = "The size of the thread pool. This pool is used to execute jobs and therefore "
+ + "limits the maximum number of jobs executed in parallel. "
+ + "A value of -1 is substituted with the number of available processors. "
+ + "A decimal number between 0.0 and 1.0 is treated as a fraction of available processors. "
+ + "For example 0.5 means half of the available processors. ")
+ double minPoolSize() default 35;
}
+ /**
+ * Logger.
+ */
+ private final Logger logger = LoggerFactory.getLogger(this.getClass());
Review Comment:
I think that in most Sling classes we use a static Logger:
```suggestion
private static final Logger LOGGER = LoggerFactory.getLogger(EventingThreadPool.class);
```
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "royteeuwen (via GitHub)" <gi...@apache.org>.
royteeuwen commented on code in PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#discussion_r1819164499
##########
src/main/java/org/apache/sling/event/impl/EventingThreadPool.java:
##########
@@ -53,11 +55,19 @@ public class EventingThreadPool implements ThreadPool {
public @interface Config {
@AttributeDefinition(name = "Pool Size",
- description="The size of the thread pool. This pool is used to execute jobs and therefore "
- + "limits the maximum number of jobs executed in parallel.")
- int minPoolSize() default 35;
+ description = "The size of the thread pool. This pool is used to execute jobs and therefore "
+ + "limits the maximum number of jobs executed in parallel. "
+ + "A value of -1 is substituted with the number of available processors. "
+ + "A decimal number between 0.0 and 1.0 is treated as a fraction of available processors. "
+ + "For example 0.5 means half of the available processors. ")
+ double minPoolSize() default 35;
}
+ /**
+ * Logger.
+ */
+ private final Logger logger = LoggerFactory.getLogger(this.getClass());
Review Comment:
Done
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#issuecomment-2442552553
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-event&pullRequest=38) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-event&pullRequest=38&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-event&pullRequest=38&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-event&pullRequest=38&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [82.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-event&pullRequest=38&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-event&pullRequest=38&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-event&pullRequest=38)
--
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: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#discussion_r1819164641
##########
src/main/java/org/apache/sling/event/impl/EventingThreadPool.java:
##########
@@ -53,11 +55,19 @@ public class EventingThreadPool implements ThreadPool {
public @interface Config {
@AttributeDefinition(name = "Pool Size",
- description="The size of the thread pool. This pool is used to execute jobs and therefore "
- + "limits the maximum number of jobs executed in parallel.")
- int minPoolSize() default 35;
+ description = "The size of the thread pool. This pool is used to execute jobs and therefore "
+ + "limits the maximum number of jobs executed in parallel. "
+ + "A value of -1 is substituted with the number of available processors. "
+ + "A decimal number between 0.0 and 1.0 is treated as a fraction of available processors. "
+ + "For example 0.5 means half of the available processors. ")
+ double minPoolSize() default 35;
Review Comment:
It is according to https://docs.osgi.org/specification/osgi.cmpn/8.0.0/service.component.html#service.component-component.property.value.coercion as it uses `Integer.doubleValue()`
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#issuecomment-2437498997
@royteeuwen Do you have some time to pick up the comments from @joerghoh ? Would be great to merge this in.
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on code in PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#discussion_r1819164641
##########
src/main/java/org/apache/sling/event/impl/EventingThreadPool.java:
##########
@@ -53,11 +55,19 @@ public class EventingThreadPool implements ThreadPool {
public @interface Config {
@AttributeDefinition(name = "Pool Size",
- description="The size of the thread pool. This pool is used to execute jobs and therefore "
- + "limits the maximum number of jobs executed in parallel.")
- int minPoolSize() default 35;
+ description = "The size of the thread pool. This pool is used to execute jobs and therefore "
+ + "limits the maximum number of jobs executed in parallel. "
+ + "A value of -1 is substituted with the number of available processors. "
+ + "A decimal number between 0.0 and 1.0 is treated as a fraction of available processors. "
+ + "For example 0.5 means half of the available processors. ")
+ double minPoolSize() default 35;
Review Comment:
It is according to https://docs.osgi.org/specification/osgi.cmpn/8.0.0/service.component.html#service.component-component.property.value.coercion.
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "royteeuwen (via GitHub)" <gi...@apache.org>.
royteeuwen commented on PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#issuecomment-2273383715
The build still seems to fail, on Java 11 linux, while the same issue does not occur for me on Java 11... Will have to investigate further
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "royteeuwen (via GitHub)" <gi...@apache.org>.
royteeuwen commented on code in PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#discussion_r1819165480
##########
src/main/java/org/apache/sling/event/impl/EventingThreadPool.java:
##########
@@ -53,11 +55,19 @@ public class EventingThreadPool implements ThreadPool {
public @interface Config {
@AttributeDefinition(name = "Pool Size",
- description="The size of the thread pool. This pool is used to execute jobs and therefore "
- + "limits the maximum number of jobs executed in parallel.")
- int minPoolSize() default 35;
+ description = "The size of the thread pool. This pool is used to execute jobs and therefore "
+ + "limits the maximum number of jobs executed in parallel. "
+ + "A value of -1 is substituted with the number of available processors. "
+ + "A decimal number between 0.0 and 1.0 is treated as a fraction of available processors. "
+ + "For example 0.5 means half of the available processors. ")
+ double minPoolSize() default 35;
Review Comment:
Yep, just tested it out and it works:
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "royteeuwen (via GitHub)" <gi...@apache.org>.
royteeuwen commented on PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#issuecomment-2441824118
> @royteeuwen Do you have some time to pick up the comments from @joerghoh ? Would be great to merge this in.
Resolved and answered his comments.
I also removed the update of the parent of this PR and just made the PR build on JDK11 only. Still it seems that the windows 11 build fails, only the linux one succeeds so far. (don't think its related to my PR though). Any suggestions on going forward?
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add processor based calculation for the eventing threadpool [sling-org-apache-sling-event]
Posted by "kwin (via GitHub)" <gi...@apache.org>.
kwin commented on PR #38:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/38#issuecomment-2442234511
We probably need to get the ITs stable first, I made an attempt in https://github.com/apache/sling-org-apache-sling-event/pull/37.
--
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: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org