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

Andrew Wang commented on HDFS-5394:
-----------------------------------

Hey Colin, thanks for the responding promptly,

bq. I think the state machine makes things easier to understand, and is more 
efficient as well...

I think my proposal really isn't that different conceptually from what's 
already there. It's essentially a simple dataflow pipeline: we have work queues 
(the pending maps) that feed workers (the cache/uncache executors). I think 
having a few maps is very clear and self-evident.

What we have now isn't pure state machine enough to make me happy. This style 
works best when the state object has all the transition logic, with very well 
defined transitions. Here, we have a bunch of different methods reaching into 
the replicasMap and doing things with the MappableBlocks based on external 
input and shared data, which is not pure. Also, like I said before, this is 
pretty simple state transition graph, so it's not like the state machine is 
helping clarify recursive behavior or something.

The efficiency argument isn't convincing to me, since the swap/retry stuff does 
complicate the code when this isn't even a hot path. 

bq. Whether to advertise a block is a different issue than state machine vs. ad 
hoc...

I was trying to say that this would be more concise with a dedicated 
{{cachedReplicas}} map.

bq. It's not really possible for Uncaching tasks to "get stuck" currently, 
since all they do is munmap...In the long term, we are not going to have 
threads blocking waiting for clients.

I was thinking of when we are trying to do client revocation, where a client 
could stall the uncaching. Good point about the reverse situation too though, 
thanks for pulling the executors apart.

bq. Instead, we'll have tasks that reschedule themselves with some fixed period 
to poll whether the client has released the mmap.

This sounds kind of like my background sweeper thread idea. I don't think we 
need a whole thread pool to do the munmaps since (as you said) it should be 
basically free if it's ready to be uncached.

bq. bq. Why is it called "visibleLength" instead of just just "length"? 

Could we do that rename to "visibleLength" when we actually support said 
functionality? :) thanks

> fix race conditions in DN caching and uncaching
> -----------------------------------------------
>
>                 Key: HDFS-5394
>                 URL: https://issues.apache.org/jira/browse/HDFS-5394
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: datanode, namenode
>    Affects Versions: HDFS-4949
>            Reporter: Colin Patrick McCabe
>            Assignee: Colin Patrick McCabe
>         Attachments: HDFS-5394-caching.001.patch, 
> HDFS-5394-caching.002.patch, HDFS-5394-caching.003.patch, 
> HDFS-5394-caching.004.patch
>
>
> The DN needs to handle situations where it is asked to cache the same replica 
> more than once.  (Currently, it can actually do two mmaps and mlocks.)  It 
> also needs to handle the situation where caching a replica is cancelled 
> before said caching completes.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Reply via email to