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