You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Sergey Shelukhin <se...@hortonworks.com> on 2016/07/06 21:31:48 UTC
Review Request 49728: HIVE-14172 LLAP: force evict blocks by size to
handle memory fragmentation
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49728/
-----------------------------------------------------------
Review request for hive and Gopal V.
Repository: hive-git
Description
-------
see jira
Diffs
-----
llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java 47325ad
llap-server/src/java/org/apache/hadoop/hive/llap/cache/LlapCacheableBuffer.java 5c0b6f3
llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheMemoryManager.java 4def4a1
llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCachePolicy.java acbaf85
llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelFifoCachePolicy.java 0838682
llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java 5a0b27f
llap-server/src/java/org/apache/hadoop/hive/llap/cache/MemoryManager.java 6cc262e
llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestBuddyAllocator.java 345f5b1
llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelCacheImpl.java 0846db9
llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelLrfuCachePolicy.java 616c040
llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java 40edb28
Diff: https://reviews.apache.org/r/49728/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 49728: HIVE-14172 LLAP: force evict blocks by size
to handle memory fragmentation
Posted by Gopal V <go...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49728/#review142109
-----------------------------------------------------------
Ship it!
llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java (line 228)
<https://reviews.apache.org/r/49728/#comment207605>
From my paraphrashing.
The new sublist extracted from the locked list will not be accessible to any future traversals.
The spliced sub-list disappears for future reads, which is how this does not have a race condition.
- Gopal V
On July 6, 2016, 9:31 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49728/
> -----------------------------------------------------------
>
> (Updated July 6, 2016, 9:31 p.m.)
>
>
> Review request for hive and Gopal V.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java 47325ad
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LlapCacheableBuffer.java 5c0b6f3
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheMemoryManager.java 4def4a1
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCachePolicy.java acbaf85
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelFifoCachePolicy.java 0838682
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java 5a0b27f
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/MemoryManager.java 6cc262e
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestBuddyAllocator.java 345f5b1
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelCacheImpl.java 0846db9
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelLrfuCachePolicy.java 616c040
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java 40edb28
>
> Diff: https://reviews.apache.org/r/49728/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 49728: HIVE-14172 LLAP: force evict blocks by size
to handle memory fragmentation
Posted by Sergey Shelukhin <se...@hortonworks.com>.
> On July 13, 2016, 4:54 p.m., Gopal V wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java, line 229
> > <https://reviews.apache.org/r/49728/diff/1/?file=1438086#file1438086line229>
> >
> > Race condition?
hmm? :)
- Sergey
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49728/#review142076
-----------------------------------------------------------
On July 6, 2016, 9:31 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49728/
> -----------------------------------------------------------
>
> (Updated July 6, 2016, 9:31 p.m.)
>
>
> Review request for hive and Gopal V.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java 47325ad
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LlapCacheableBuffer.java 5c0b6f3
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheMemoryManager.java 4def4a1
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCachePolicy.java acbaf85
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelFifoCachePolicy.java 0838682
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java 5a0b27f
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/MemoryManager.java 6cc262e
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestBuddyAllocator.java 345f5b1
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelCacheImpl.java 0846db9
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelLrfuCachePolicy.java 616c040
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java 40edb28
>
> Diff: https://reviews.apache.org/r/49728/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 49728: HIVE-14172 LLAP: force evict blocks by size
to handle memory fragmentation
Posted by Sergey Shelukhin <se...@hortonworks.com>.
> On July 13, 2016, 4:54 p.m., Gopal V wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java, line 227
> > <https://reviews.apache.org/r/49728/diff/1/?file=1438086#file1438086line227>
> >
> > The assumption about dropping locked blocks - does it ever happen that another thread has a valid reference to that block when this is hit?
LRFU policy does not need references for locked blocks because unlocking the block adds it back to the list/heap. However, this (removal) is not strictly enforced to avoid waiting/traversing when locking ie from processing thread every time. So the thread that already has to traverse removes them. It's not an eviction (eviction calls invalidate())
- Sergey
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49728/#review142076
-----------------------------------------------------------
On July 6, 2016, 9:31 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49728/
> -----------------------------------------------------------
>
> (Updated July 6, 2016, 9:31 p.m.)
>
>
> Review request for hive and Gopal V.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java 47325ad
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LlapCacheableBuffer.java 5c0b6f3
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheMemoryManager.java 4def4a1
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCachePolicy.java acbaf85
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelFifoCachePolicy.java 0838682
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java 5a0b27f
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/MemoryManager.java 6cc262e
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestBuddyAllocator.java 345f5b1
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelCacheImpl.java 0846db9
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelLrfuCachePolicy.java 616c040
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java 40edb28
>
> Diff: https://reviews.apache.org/r/49728/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 49728: HIVE-14172 LLAP: force evict blocks by size
to handle memory fragmentation
Posted by Gopal V <go...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49728/#review142076
-----------------------------------------------------------
llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java (line 226)
<https://reviews.apache.org/r/49728/#comment207571>
The assumption about dropping locked blocks - does it ever happen that another thread has a valid reference to that block when this is hit?
llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java (line 228)
<https://reviews.apache.org/r/49728/#comment207572>
Race condition?
- Gopal V
On July 6, 2016, 9:31 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49728/
> -----------------------------------------------------------
>
> (Updated July 6, 2016, 9:31 p.m.)
>
>
> Review request for hive and Gopal V.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java 47325ad
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LlapCacheableBuffer.java 5c0b6f3
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheMemoryManager.java 4def4a1
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCachePolicy.java acbaf85
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelFifoCachePolicy.java 0838682
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java 5a0b27f
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/MemoryManager.java 6cc262e
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestBuddyAllocator.java 345f5b1
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelCacheImpl.java 0846db9
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelLrfuCachePolicy.java 616c040
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java 40edb28
>
> Diff: https://reviews.apache.org/r/49728/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 49728: HIVE-14172 LLAP: force evict blocks by size
to handle memory fragmentation
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49728/#review141080
-----------------------------------------------------------
llap-server/src/java/org/apache/hadoop/hive/llap/cache/LlapCacheableBuffer.java (line 52)
<https://reviews.apache.org/r/49728/#comment206467>
bogus
llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheMemoryManager.java (line 119)
<https://reviews.apache.org/r/49728/#comment206468>
also bogus :)
- Sergey Shelukhin
On July 6, 2016, 9:31 p.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49728/
> -----------------------------------------------------------
>
> (Updated July 6, 2016, 9:31 p.m.)
>
>
> Review request for hive and Gopal V.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> see jira
>
>
> Diffs
> -----
>
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java 47325ad
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LlapCacheableBuffer.java 5c0b6f3
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheMemoryManager.java 4def4a1
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCachePolicy.java acbaf85
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelFifoCachePolicy.java 0838682
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java 5a0b27f
> llap-server/src/java/org/apache/hadoop/hive/llap/cache/MemoryManager.java 6cc262e
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestBuddyAllocator.java 345f5b1
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelCacheImpl.java 0846db9
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelLrfuCachePolicy.java 616c040
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java 40edb28
>
> Diff: https://reviews.apache.org/r/49728/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>