You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@jena.apache.org by GitBox <gi...@apache.org> on 2020/03/27 08:21:49 UTC
[GitHub] [jena] afs opened a new pull request #718: JENA-1871: Check and
limit the interval for ActionSleep.
afs opened a new pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718
Fixes for:
[JENA-1871](https://issues.apache.org/jira/browse/JENA-1871) -- ActionSleep
[JENA-1872](https://issues.apache.org/jira/browse/JENA-1872) -- async task pool
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #718:
JENA-1871: Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399159125
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/ctl/ActionSleep.java
##########
@@ -32,6 +32,8 @@
/** A task that kicks off a asynchronous operation that simply waits and exits. For testing. */
public class ActionSleep extends ActionCtl /* Not ActionAsyncTask - that is a container-item based. */
{
+ private static final int MaxSleepMillis = 20*1000;
Review comment:
Could we make this public then the test case can refer to the constant rather than hard-coding it
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #718: JENA-1871:
Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399539503
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/async/AsyncPool.java
##########
@@ -34,10 +35,16 @@
// Number of finished tasks kept.
private static int MAX_FINISHED = 20;
- // See Executors.newCachedThreadPool and Executors.newFixedThreadPool
+ // A ThreadPoolExecutor with
+ // * 0 to nMaxThreads
+ // * no queue of waiting tasks (tasks execute or are rejected)
+ // * dormant threads released after 120s.
+ //
+ // SynchronousQueue is a BlockingQueue that has zero length - it accepts and
+ // delivers an item or rejects immediately, no delay by queueing.
Review comment:
Yes - like that. The actual mechanism is expands up to nMaxThreads, creating threads if needed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #718:
JENA-1871: Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399159481
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/test/java/org/apache/jena/fuseki/TestAdmin.java
##########
@@ -278,7 +278,29 @@
deleteDataset(dsTest);
}
- // Sync task testing
+ @Test public void sleep_1() {
+ String x = execSleepTask(null, 1);
+ }
+
+ @Test public void sleep_2() {
+ try {
+ String x = execSleepTask(null, -1);
+ fail("Sleep call unexpectedly succeed");
+ } catch (HttpException ex) {
+ assertEquals(400, ex.getStatusCode());
+ }
+ }
+
+ @Test public void sleep_3() {
+ try {
+ String x = execSleepTask(null, 20*1000+1);
Review comment:
Would be nicer if this just referred to the constant in `ActionSleep` so if we ever change that the test doesn't break
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] kinow commented on a change in pull request #718: JENA-1871:
Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399101415
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/async/AsyncPool.java
##########
@@ -34,10 +35,16 @@
// Number of finished tasks kept.
private static int MAX_FINISHED = 20;
- // See Executors.newCachedThreadPool and Executors.newFixedThreadPool
+ // A ThreadPoolExecutor with
+ // * 0 to nMaxThreads
+ // * no queue of waiting tasks 9tasks execute or are rejected)
Review comment:
s/9tasks/(tasks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #718: JENA-1871:
Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399651945
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/test/java/org/apache/jena/fuseki/TestAdmin.java
##########
@@ -278,7 +278,29 @@
deleteDataset(dsTest);
}
- // Sync task testing
+ @Test public void sleep_1() {
+ String x = execSleepTask(null, 1);
+ }
+
+ @Test public void sleep_2() {
+ try {
+ String x = execSleepTask(null, -1);
+ fail("Sleep call unexpectedly succeed");
+ } catch (HttpException ex) {
+ assertEquals(400, ex.getStatusCode());
+ }
+ }
+
+ @Test public void sleep_3() {
+ try {
+ String x = execSleepTask(null, 20*1000+1);
+ fail("Sleep call unexpectedly succeed");
+ } catch (HttpException ex) {
+ assertEquals(400, ex.getStatusCode());
+ }
+ }
+
+ // Async task testing
Review comment:
I've pushed two tests.
If they become unreliable in practice, I'd prefer that we comment out the `@Test`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #718: JENA-1871:
Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399538894
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/async/AsyncPool.java
##########
@@ -63,9 +70,14 @@ public AsyncTask submit(Runnable task, String displayName, DataService dataServi
return null;
};
AsyncTask asyncTask = new AsyncTask(c, this, taskId, displayName, dataService, requestId);
- /* Future<Object> future = */ executor.submit(asyncTask);
- runningTasks.put(taskId, asyncTask);
- return asyncTask;
+ try {
+ /* Future<Object> future = */ executor.submit(asyncTask);
+ runningTasks.put(taskId, asyncTask);
+ return asyncTask;
+ } catch (RejectedExecutionException ex) {
+ ServletOps.errorBadRequest("Async task request rejected");
Review comment:
OK
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #718: JENA-1871:
Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399153985
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/ctl/ActionSleep.java
##########
@@ -32,6 +32,8 @@
/** A task that kicks off a asynchronous operation that simply waits and exits. For testing. */
public class ActionSleep extends ActionCtl /* Not ActionAsyncTask - that is a container-item based. */
{
+ private static int MaxSleepMillis = 20*1000;
Review comment:
Logically, it's a constant. I'll add `final`. ALL_CAPS_IS_A_BIT_SHOUTY :-)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #718: JENA-1871:
Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399135084
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/ctl/ActionSleep.java
##########
@@ -50,45 +52,57 @@ public void validate(HttpAction action) {}
@Override
public void execute(HttpAction action) {
- Runnable task = createRunnable(action);
+ SleepTask task = createRunnable(action);
AsyncTask aTask = Async.execASyncTask(action, AsyncPool.get(), "sleep", task);
+ action.log.info(format("[%d] Sleep %d ms.", action.id, task.sleepMilli));
JsonValue v = Async.asJson(aTask);
- Async.setLocationHeader(action, aTask);
ServletOps.sendJsonReponse(action, v);
}
- protected Runnable createRunnable(HttpAction action) {
+ protected SleepTask createRunnable(HttpAction action) {
String interval = action.request.getParameter("interval");
int sleepMilli = 5000;
- if ( interval != null )
+ if ( interval != null ) {
try {
sleepMilli = Integer.parseInt(interval);
} catch (NumberFormatException ex) {
- action.log.error(format("[%d] NumberFormatException: %s", action.id, interval));
+ ServletOps.errorBadRequest("Bad format for 'interval': integer required");
Review comment:
Good idea.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs merged pull request #718: JENA-1871: Check and limit the
interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
afs merged pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] kinow commented on a change in pull request #718: JENA-1871:
Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399102180
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/ctl/ActionSleep.java
##########
@@ -32,6 +32,8 @@
/** A task that kicks off a asynchronous operation that simply waits and exits. For testing. */
public class ActionSleep extends ActionCtl /* Not ActionAsyncTask - that is a container-item based. */
{
+ private static int MaxSleepMillis = 20*1000;
Review comment:
`maxSleepMillis`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] kinow commented on a change in pull request #718: JENA-1871:
Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399107311
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/ctl/ActionSleep.java
##########
@@ -50,45 +52,57 @@ public void validate(HttpAction action) {}
@Override
public void execute(HttpAction action) {
- Runnable task = createRunnable(action);
+ SleepTask task = createRunnable(action);
AsyncTask aTask = Async.execASyncTask(action, AsyncPool.get(), "sleep", task);
+ action.log.info(format("[%d] Sleep %d ms.", action.id, task.sleepMilli));
JsonValue v = Async.asJson(aTask);
- Async.setLocationHeader(action, aTask);
ServletOps.sendJsonReponse(action, v);
}
- protected Runnable createRunnable(HttpAction action) {
+ protected SleepTask createRunnable(HttpAction action) {
String interval = action.request.getParameter("interval");
int sleepMilli = 5000;
- if ( interval != null )
+ if ( interval != null ) {
try {
sleepMilli = Integer.parseInt(interval);
} catch (NumberFormatException ex) {
- action.log.error(format("[%d] NumberFormatException: %s", action.id, interval));
+ ServletOps.errorBadRequest("Bad format for 'interval': integer required");
+ return null;
}
- action.log.info(format("[%d] Sleep %d ms", action.id, sleepMilli));
- return new SleepTask(action, sleepMilli);
+ }
+ if ( sleepMilli < 0 ) {
+ ServletOps.errorBadRequest("Negative sleep interval");
+ return null;
+ }
+ if ( sleepMilli > MaxSleepMillis ) {
+ ServletOps.errorBadRequest("Sleep internal greater than maximum allowed");
+ return null;
+ }
+ return new SleepTask(action, sleepMilli, AsyncPool.get());
}
static class SleepTask implements Runnable {
private final Logger log;
private final long actionId;
- private final int sleepMilli;
+ public final int sleepMilli;
+ private final AsyncPool asyncPool;
- public SleepTask(HttpAction action, int sleepMilli ) {
+ public SleepTask(HttpAction action, int sleepMilli, AsyncPool asyncPool ) {
this.log = action.log;
this.actionId = action.id;
this.sleepMilli = sleepMilli;
+ this.asyncPool = asyncPool;
}
@Override
public void run() {
try {
- log.info(format("[%d] >> Sleep start", actionId));
- Lib.sleep(sleepMilli);
- log.info(format("[%d] << Sleep finish", actionId));
+ log.info(format("[Task %d] >> Sleep start", actionId));
+ for ( int i = 0 ; i < 10 ; i++ )
+ Lib.sleep(sleepMilli/10);
Review comment:
Out of curiosity, why `/ 10`? Is it to yield control so other things get a chance to run after every 1/10th of the sleep time?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #718: JENA-1871:
Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399135053
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/ctl/ActionSleep.java
##########
@@ -50,45 +52,57 @@ public void validate(HttpAction action) {}
@Override
public void execute(HttpAction action) {
- Runnable task = createRunnable(action);
+ SleepTask task = createRunnable(action);
AsyncTask aTask = Async.execASyncTask(action, AsyncPool.get(), "sleep", task);
+ action.log.info(format("[%d] Sleep %d ms.", action.id, task.sleepMilli));
JsonValue v = Async.asJson(aTask);
- Async.setLocationHeader(action, aTask);
ServletOps.sendJsonReponse(action, v);
}
- protected Runnable createRunnable(HttpAction action) {
+ protected SleepTask createRunnable(HttpAction action) {
String interval = action.request.getParameter("interval");
int sleepMilli = 5000;
- if ( interval != null )
+ if ( interval != null ) {
try {
sleepMilli = Integer.parseInt(interval);
} catch (NumberFormatException ex) {
- action.log.error(format("[%d] NumberFormatException: %s", action.id, interval));
+ ServletOps.errorBadRequest("Bad format for 'interval': integer required");
+ return null;
}
- action.log.info(format("[%d] Sleep %d ms", action.id, sleepMilli));
- return new SleepTask(action, sleepMilli);
+ }
+ if ( sleepMilli < 0 ) {
+ ServletOps.errorBadRequest("Negative sleep interval");
+ return null;
+ }
+ if ( sleepMilli > MaxSleepMillis ) {
+ ServletOps.errorBadRequest("Sleep internal greater than maximum allowed");
+ return null;
+ }
+ return new SleepTask(action, sleepMilli, AsyncPool.get());
}
static class SleepTask implements Runnable {
private final Logger log;
private final long actionId;
- private final int sleepMilli;
+ public final int sleepMilli;
+ private final AsyncPool asyncPool;
- public SleepTask(HttpAction action, int sleepMilli ) {
+ public SleepTask(HttpAction action, int sleepMilli, AsyncPool asyncPool ) {
this.log = action.log;
this.actionId = action.id;
this.sleepMilli = sleepMilli;
+ this.asyncPool = asyncPool;
}
@Override
public void run() {
try {
- log.info(format("[%d] >> Sleep start", actionId));
- Lib.sleep(sleepMilli);
- log.info(format("[%d] << Sleep finish", actionId));
+ log.info(format("[Task %d] >> Sleep start", actionId));
+ for ( int i = 0 ; i < 10 ; i++ )
+ Lib.sleep(sleepMilli/10);
Review comment:
Yes - that's the reason. It is dev code put in to nudge the JVM along when running all in in JVM (client and Fuseki embedded server). I find that it sometimes shows up concurrency issues better and I was trying multiple "sleeps" running.
It isn't supposed to be there for the codebase and I'll remove it - good thing we have code review.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] kinow commented on a change in pull request #718: JENA-1871:
Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
kinow commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399103043
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/ctl/ActionSleep.java
##########
@@ -50,45 +52,57 @@ public void validate(HttpAction action) {}
@Override
public void execute(HttpAction action) {
- Runnable task = createRunnable(action);
+ SleepTask task = createRunnable(action);
AsyncTask aTask = Async.execASyncTask(action, AsyncPool.get(), "sleep", task);
+ action.log.info(format("[%d] Sleep %d ms.", action.id, task.sleepMilli));
JsonValue v = Async.asJson(aTask);
- Async.setLocationHeader(action, aTask);
ServletOps.sendJsonReponse(action, v);
}
- protected Runnable createRunnable(HttpAction action) {
+ protected SleepTask createRunnable(HttpAction action) {
String interval = action.request.getParameter("interval");
int sleepMilli = 5000;
- if ( interval != null )
+ if ( interval != null ) {
try {
sleepMilli = Integer.parseInt(interval);
} catch (NumberFormatException ex) {
- action.log.error(format("[%d] NumberFormatException: %s", action.id, interval));
+ ServletOps.errorBadRequest("Bad format for 'interval': integer required");
Review comment:
Maybe use the same prefix for all errors?
`"Bad format for 'interval': integer required"`
`"Bad format for 'interval': negative sleep interval"`
`"Bad format for 'interval': sleep internal greater than maximum allowed"`
?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #718:
JENA-1871: Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399160428
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/async/AsyncPool.java
##########
@@ -34,10 +35,16 @@
// Number of finished tasks kept.
private static int MAX_FINISHED = 20;
- // See Executors.newCachedThreadPool and Executors.newFixedThreadPool
+ // A ThreadPoolExecutor with
+ // * 0 to nMaxThreads
+ // * no queue of waiting tasks (tasks execute or are rejected)
+ // * dormant threads released after 120s.
+ //
+ // SynchronousQueue is a BlockingQueue that has zero length - it accepts and
+ // delivers an item or rejects immediately, no delay by queueing.
Review comment:
+1 So provided there is at least one thread free then submitting a task will succeed, if all threads are in-use then it fails
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #718:
JENA-1871: Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399160963
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/async/AsyncPool.java
##########
@@ -63,9 +70,14 @@ public AsyncTask submit(Runnable task, String displayName, DataService dataServi
return null;
};
AsyncTask asyncTask = new AsyncTask(c, this, taskId, displayName, dataService, requestId);
- /* Future<Object> future = */ executor.submit(asyncTask);
- runningTasks.put(taskId, asyncTask);
- return asyncTask;
+ try {
+ /* Future<Object> future = */ executor.submit(asyncTask);
+ runningTasks.put(taskId, asyncTask);
+ return asyncTask;
+ } catch (RejectedExecutionException ex) {
+ ServletOps.errorBadRequest("Async task request rejected");
Review comment:
Should this be more specific so users understand why it was rejected e.g. add something like ` - already <maxTasks> in progress`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #718: JENA-1871:
Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399536556
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/test/java/org/apache/jena/fuseki/TestAdmin.java
##########
@@ -278,7 +278,29 @@
deleteDataset(dsTest);
}
- // Sync task testing
+ @Test public void sleep_1() {
+ String x = execSleepTask(null, 1);
+ }
+
+ @Test public void sleep_2() {
+ try {
+ String x = execSleepTask(null, -1);
+ fail("Sleep call unexpectedly succeed");
+ } catch (HttpException ex) {
+ assertEquals(400, ex.getStatusCode());
+ }
+ }
+
+ @Test public void sleep_3() {
+ try {
+ String x = execSleepTask(null, 20*1000+1);
+ fail("Sleep call unexpectedly succeed");
+ } catch (HttpException ex) {
+ assertEquals(400, ex.getStatusCode());
+ }
+ }
+
+ // Async task testing
Review comment:
Tricky. Any ideas how to?
I can investigate reading the tasks list for active tasks but we have situations where loaded CI servers do weird things (pauses of 10s of seconds or longer - I guess the VM isn't get real CPU time) so "overlap " is hard to ensure in a reliable way, including with the client code in the `@Test` itself.
Having "works mostly" is bad - it it then natural to ignore test failures because they become "expected".
I have tested locally, including hitting the limit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #718: JENA-1871:
Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399534351
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/ctl/ActionSleep.java
##########
@@ -32,6 +32,8 @@
/** A task that kicks off a asynchronous operation that simply waits and exits. For testing. */
public class ActionSleep extends ActionCtl /* Not ActionAsyncTask - that is a container-item based. */
{
+ private static final int MaxSleepMillis = 20*1000;
Review comment:
OK
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #718: JENA-1871:
Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399154019
##########
File path: jena-fuseki2/jena-fuseki-core/src/main/java/org/apache/jena/fuseki/async/AsyncPool.java
##########
@@ -34,10 +35,16 @@
// Number of finished tasks kept.
private static int MAX_FINISHED = 20;
- // See Executors.newCachedThreadPool and Executors.newFixedThreadPool
+ // A ThreadPoolExecutor with
+ // * 0 to nMaxThreads
+ // * no queue of waiting tasks 9tasks execute or are rejected)
Review comment:
Will fix!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #718:
JENA-1871: Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r400071729
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/test/java/org/apache/jena/fuseki/TestAdmin.java
##########
@@ -278,7 +278,29 @@
deleteDataset(dsTest);
}
- // Sync task testing
+ @Test public void sleep_1() {
+ String x = execSleepTask(null, 1);
+ }
+
+ @Test public void sleep_2() {
+ try {
+ String x = execSleepTask(null, -1);
+ fail("Sleep call unexpectedly succeed");
+ } catch (HttpException ex) {
+ assertEquals(400, ex.getStatusCode());
+ }
+ }
+
+ @Test public void sleep_3() {
+ try {
+ String x = execSleepTask(null, 20*1000+1);
+ fail("Sleep call unexpectedly succeed");
+ } catch (HttpException ex) {
+ assertEquals(400, ex.getStatusCode());
+ }
+ }
+
+ // Async task testing
Review comment:
LGTM
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #718:
JENA-1871: Check and limit the interval for ActionSleep.
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #718: JENA-1871: Check and limit the interval for ActionSleep.
URL: https://github.com/apache/jena/pull/718#discussion_r399161884
##########
File path: jena-fuseki2/jena-fuseki-webapp/src/test/java/org/apache/jena/fuseki/TestAdmin.java
##########
@@ -278,7 +278,29 @@
deleteDataset(dsTest);
}
- // Sync task testing
+ @Test public void sleep_1() {
+ String x = execSleepTask(null, 1);
+ }
+
+ @Test public void sleep_2() {
+ try {
+ String x = execSleepTask(null, -1);
+ fail("Sleep call unexpectedly succeed");
+ } catch (HttpException ex) {
+ assertEquals(400, ex.getStatusCode());
+ }
+ }
+
+ @Test public void sleep_3() {
+ try {
+ String x = execSleepTask(null, 20*1000+1);
+ fail("Sleep call unexpectedly succeed");
+ } catch (HttpException ex) {
+ assertEquals(400, ex.getStatusCode());
+ }
+ }
+
+ // Async task testing
Review comment:
Do we want some additional test cases that validate that we can now have multiple simultaneous async tasks? I see test cases that cover JENA-1871 but not any that cover JENA-1872
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org