[jira] [Commented] (HDFS-5950) The DFSClient and DataNode should use shared memory segments to communicate short-circuit information

["Colin Patrick McCabe (JIRA)" <ji...@apache.org>]

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

Colin Patrick McCabe commented on HDFS-5950:
--------------------------------------------

bq. Add new config keys to hdfs-default.xml

Added

bq. 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.

I'll file a follow-on JIRA.  We need some way of timing out misbehaving clients, so I'll throw this in there.

bq. 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.

An array of size 2 has 16 bytes of object overhead, plus another 16 of actual data.  So it would increase the size by at least 2x.  See http://www.javaworld.com/article/2077496/testing-debugging/java-tip-130--do-you-know-your-data-size-.html.  It's simpler and more efficient just to use two longs, since we know the size here...

bq. Constructor is doing the same precondition check twice

fixed

bq. 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.

It's not an issue, because T2 can't run until we release the lock-- either by exiting allocSlot, or by retrying the loading.

bq. 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.

The assumption here was that the slot would never be stale unless either the DN or the DFSClient had closed the associated UNIX domain socket (and hence the handler had run or was about to run.)  Maybe I should check whether it has been removed anyway, though...

bq. Can put @VisibleForTesting on the Visitor interface

ok

bq. pullSlot isn't very descriptive, maybe allocSlotFromExistingShm?

ok

bq. In the requestFileDescriptors path, I don't see where slot == null is handled (when a DN does not support shm segments).

It's handled here:
{code}
    SlotId slotId = slot == null ? null : slot.getSlotId();
{code}

If slotId == null, we don't fill in those optional PB fields and things work like previously.  

bq. Is there a test for this?

There's a test in {{TestBlockReaderFactory#testShortCircuitReadFromServerWithoutShm}}.  There's also a test for if the client doesn't want shm and the server has it.

bq. [ShortCircuitRegistry javadoc]

I modified this a bit and added it, thanks.

bq. Slot#VALID_FLAG:

This one already has javadoc which I kind of like... I improved the existing javadoc a bit

bq. DfsClientShmManager

Added, thanks.

> 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)