[ 
https://issues.apache.org/jira/browse/HADOOP-11684?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14961766#comment-14961766
 ] 

Lei (Eddy) Xu commented on HADOOP-11684:
----------------------------------------

Thanks a lot for working on this, [~Thomas Demoor] and [~fabbri]. It looks good 
overall.

I have a few nitpicks, would +1 after addressing them:

* {{BlockingThreadPoolExecutorServiceTest}} should be renamed to 
{{TestBlockingThreadPoolExecutorService}}.
* Could you consider not throwing {{RuntimeException}} in the 
BlockingThreadPoolExecutorServiceTest? I.e. , we might want to change 
{code}
if (sw.now(TimeUnit.MILLISECONDS) > BLOCKING_THRESHOLD_MSEC) {
  throw new RuntimeException("Non-blocking call took too long.");
}
{code} to
{code}
assertTrue("Non-blocking call took too long.", sw.now(...) > BLOCK...)
{code}
* Should we expect the "        import static org.junit.Assert.*;" ?
* Lets change {{int shutdown_tries = SHUTDOWN_WAIT_TRIES;}} to {{int 
shutdownTries = ...}} for code style consistency. 
* Use {{sleeper}} to test blocking the threadpool might be little bit flaky 
from time to time. Could you consider to use a conditional variable or latch to 
block the active tasks?

Thanks a lot!

> S3a to use thread pool that blocks clients
> ------------------------------------------
>
>                 Key: HADOOP-11684
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11684
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: fs/s3
>    Affects Versions: 2.7.0
>            Reporter: Thomas Demoor
>            Assignee: Thomas Demoor
>         Attachments: HADOOP-11684-001.patch, HADOOP-11684-002.patch, 
> HADOOP-11684-003.patch, HADOOP-11684-004.patch
>
>
> Currently, if fs.s3a.max.total.tasks are queued and another (part)upload 
> wants to start, a RejectedExecutionException is thrown. 
> We should use a threadpool that blocks clients, nicely throtthling them, 
> rather than throwing an exception. F.i. something similar to 
> https://github.com/apache/incubator-s4/blob/master/subprojects/s4-comm/src/main/java/org/apache/s4/comm/staging/BlockingThreadPoolExecutorService.java



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to