[
https://issues.apache.org/jira/browse/HDFS-5950?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13916396#comment-13916396
]
Andrew Wang commented on HDFS-5950:
-----------------------------------
Nice work, looks like another meaty patch. I didn't do a super-detailed review
like last time, but overall it looks good and I think we should just get this
checked in and start hammering on it.
General:
- Add new config keys to hdfs-default.xml
- Maybe as a follow on, it'd be good to have metrics for # anchorable blocks, #
uncache failures because of anchoring, # of slots and shms, used slots per shm,
anything else you can think of. I think it'll be useful for debugging.
ShortCircuitShm:
- I like that there's a method named "safetyDance" :)
- Rather than having Hi and Lo longs, if all you want a 128-bit identifier, why
not just use a byte array? It might also be sufficient just to use a single
long. If we retry on collisions, 64 bits should be enough.
DomainSocketWatcher:
- Constructor is doing the same precondition check twice
DfsClientShmManager:
- Is there a potential race in allocSlot, where T1 is loading, T2 is waiting,
T1 signals, T2 wakes up and tries to alloc another one? Since we signal before
putting into notFull, this can happen. Not the end of the world, but maybe
sloppy.
- In unregisterClientShmSlot, for the {{stale && empty}} case, would help to
drop a comment about how the shm is unregistered in DfsClientShm#handle, so we
can just free when it empties.
- Can put {{@VisibleForTesting}} on the Visitor interface
- pullSlot isn't very descriptive, maybe allocSlotFromExistingShm?
- In the requestFileDescriptors path, I don't see where {{slot == null}} is
handled (when a DN does not support shm segments). Is there a test for this?
While reviewing, I also wrote some notes suitable for pasting as javadoc if you
like:
ShortCircuitRegistry:
{noformat}
Manages client short-circuit memory segments on the DataNode.
When a client initiates short-circuit read with a datanode, it requests Each
short-circuit client has a shared memory segment. When a client opens a block
for short circuit read, a new "slot" for the block is allocated in the client's
shm segment.
Slots are used to track the state of the block on the both the client and
datanode. When a datanode mlocks a block, the corresponding slots for the
blocks are marked as "anchorable". Anchorable blocks can be safely read via
zero-copy read. Doing a ZCR involves increasing the "anchored" refcount on the
slot.
When a DN needs to munlock a block, it needs to first wait for the block to be
unanchored by clients doing ZCR. The DN also marks the block's slots as
"unanchorable" to prevent additional clients from trying to ZCR.
The counterpart fo this class on the client is {@link DfsClientShmManager}.
{noformat}
Slot#VALID_FLAG:
{noformat}
Flag indicating that the slot is valid, i.e. corresponds to a replica on the
DataNode.
This flag is set by the client when it allocates a new Slot. Datanodes never
set this flag.
This flag is cleared when the slot is no longer being used, e.g. when the
client releases the corresponding file descriptors or the datanode detects that
the client's domain socket was closed.
{noformat}
DfsClientShmManager:
{noformat}
Manages short-circuit memory segments for an HDFS client.
Clients are responsible for requesting and releasing shared memory segments
used for communicating with the DataNode. The client will try to allocate new
slots in the set of existing segments, falling back to getting a new segment
from the DataNode via {@link DataTransferProtocol#requestShortCircuitFds}.
The counterpart to this class on the DataNode is {@link ShortCircuitRegistry}.
{noformat}
> The DFSClient and DataNode should use shared memory segments to communicate
> short-circuit information
> -----------------------------------------------------------------------------------------------------
>
> Key: HDFS-5950
> URL: https://issues.apache.org/jira/browse/HDFS-5950
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: datanode, hdfs-client
> Affects Versions: 2.4.0
> Reporter: Colin Patrick McCabe
> Assignee: Colin Patrick McCabe
> Attachments: HDFS-5950.001.patch, HDFS-5950.003.patch,
> HDFS-5950.004.patch, HDFS-5950.006.patch, HDFS-5950.007.patch
>
>
> The DFSClient and DataNode should use the shared memory segments and unified
> cache added in the other HDFS-5182 subtasks to communicate short-circuit
> information.
--
This message was sent by Atlassian JIRA
(v6.1.5#6160)
