[
https://issues.apache.org/jira/browse/SAMZA-896?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jagadish updated SAMZA-896:
---------------------------
Description:
The ContainerRequestState class is currently not thread-safe. The class's
methods and state variables are called from both the ContainerAllocator and the
AMRMCallback handler.
Here's an analysis I summarized by looking at the current implementation. From
the below table, getContainersOnAHost() returns the entire list of containers
to the AbstractContainerAllocator which reads it. However, the Callback thread
invokes methods that write to the allocatedContainerQueue.
{noformat}
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| Method | requestQueue | hostsToCountMap |
allocatedContainerQueue | ThreadsAccesedFrom |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| updateRequestState | write | write/read | write
| Callback, Allocator |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| addContainer | | read | write
| Callback |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| updateStateAfterAssignment | write | write | read
| Allocator |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| releaseExtraContainers | write | write | write
| Allocator |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| releaseUnstartableContainer | | |
| Allocator |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| getContainersOnAHost | | | read
| Allocator |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| getRequestsQueue | read | |
| Allocator |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
{noformat}
I'm proposing to make the ContainerRequestState class thread-safe by:
i) Returning a copy of the list (if callers need the entire list - which they
should not) for getContainersOnAHost, getRequestsQueue
ii) Expose more granular synchronized APIs for things like:
1. Getting num containers on a host.
2. Getting the number of pending requests.
3. Checking if the queue is empty etc. / peeking a queue etc.
was:
The ContainerRequestState class is currently not thread-safe. The class's
methods and state variables are called from both the ContainerAllocator and the
AMRMCallback handler.
Here's an analysis I summarized by looking at the current implementation. From
the below table, getContainersOnAHost() returns the entire list of containers
to the AbstractContainerAllocator which reads it. However, the Callback thread
invokes methods that write to the allocatedContainerQueue.
{noformat}
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| Method | requestQueue | hostsToCountMap |
allocatedContainerQueue | ThreadsAccesedFrom |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| updateRequestState | write | write/read | write
| Callback, Allocator |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| addContainer | | read | write
| Callback |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| updateStateAfterAssignment | write | write | read
| Allocator |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| releaseExtraContainers | write | write | write
| Allocator |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| releaseUnstartableContainer | | |
| Allocator |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| getContainersOnAHost | | | read
| Allocator |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
| getRequestsQueue | read | |
| Allocator |
+-----------------------------+--------------+-----------------+-------------------------+---------------------+
{noformat}
I'm proposing to make the ContainerRequestState class thread-safe by:
i) Returning a copy of the list (if callers need the entire list - which they
should not)
ii) Expose more granular synchronized APIs for things like:
1. Getting num containers on a host.
2. Getting the number of pending requests.
3. Checking if the queue is empty etc.
> Improvements to thread-safety in ContainerRequestState
> ------------------------------------------------------
>
> Key: SAMZA-896
> URL: https://issues.apache.org/jira/browse/SAMZA-896
> Project: Samza
> Issue Type: Improvement
> Reporter: Jagadish
> Assignee: Jagadish
>
> The ContainerRequestState class is currently not thread-safe. The class's
> methods and state variables are called from both the ContainerAllocator and
> the AMRMCallback handler.
> Here's an analysis I summarized by looking at the current implementation.
> From the below table, getContainersOnAHost() returns the entire list of
> containers to the AbstractContainerAllocator which reads it. However, the
> Callback thread invokes methods that write to the allocatedContainerQueue.
> {noformat}
> +-----------------------------+--------------+-----------------+-------------------------+---------------------+
> | Method | requestQueue | hostsToCountMap |
> allocatedContainerQueue | ThreadsAccesedFrom |
> +-----------------------------+--------------+-----------------+-------------------------+---------------------+
> | updateRequestState | write | write/read | write
> | Callback, Allocator |
> +-----------------------------+--------------+-----------------+-------------------------+---------------------+
> | addContainer | | read | write
> | Callback |
> +-----------------------------+--------------+-----------------+-------------------------+---------------------+
> | updateStateAfterAssignment | write | write | read
> | Allocator |
> +-----------------------------+--------------+-----------------+-------------------------+---------------------+
> | releaseExtraContainers | write | write | write
> | Allocator |
> +-----------------------------+--------------+-----------------+-------------------------+---------------------+
> | releaseUnstartableContainer | | |
> | Allocator |
> +-----------------------------+--------------+-----------------+-------------------------+---------------------+
> | getContainersOnAHost | | | read
> | Allocator |
> +-----------------------------+--------------+-----------------+-------------------------+---------------------+
> | getRequestsQueue | read | |
> | Allocator |
> +-----------------------------+--------------+-----------------+-------------------------+---------------------+
> {noformat}
> I'm proposing to make the ContainerRequestState class thread-safe by:
> i) Returning a copy of the list (if callers need the entire list - which they
> should not) for getContainersOnAHost, getRequestsQueue
> ii) Expose more granular synchronized APIs for things like:
> 1. Getting num containers on a host.
> 2. Getting the number of pending requests.
> 3. Checking if the queue is empty etc. / peeking a queue etc.
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
