[GitHub] carbondata pull request #2759: [HOTFIX] Fix NPE in LRU cache when entry from...

2018-09-27 Thread asfgit 
Github user asfgit closed the pull request at:

https://github.com/apache/carbondata/pull/2759


---

[GitHub] carbondata pull request #2759: [HOTFIX] Fix NPE in LRU cache when entry from...

2018-09-26 Thread manishgupta88 
Github user manishgupta88 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2759#discussion_r220505034
  
--- Diff: 
core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryManager.java 
---
@@ -182,10 +224,15 @@ public long getUsableMemory() {
*/
   public static MemoryBlock allocateMemoryWithRetry(long taskId, long size)
   throws MemoryException {
+return allocateMemoryWithRetry(taskId, size, false);
+  }
+
+  public static MemoryBlock allocateMemoryWithRetry(long taskId, long 
size, boolean isDriver)
--- End diff --

ok done


---

[GitHub] carbondata pull request #2759: [HOTFIX] Fix NPE in LRU cache when entry from...

2018-09-26 Thread ravipesala 
Github user ravipesala commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2759#discussion_r220500265
  
--- Diff: 
core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryManager.java 
---
@@ -182,10 +224,15 @@ public long getUsableMemory() {
*/
   public static MemoryBlock allocateMemoryWithRetry(long taskId, long size)
   throws MemoryException {
+return allocateMemoryWithRetry(taskId, size, false);
+  }
+
+  public static MemoryBlock allocateMemoryWithRetry(long taskId, long 
size, boolean isDriver)
--- End diff --

Better add overloaded method to take enum for offheap or onheap memory 
required.


---

[GitHub] carbondata pull request #2759: [HOTFIX] Fix NPE in LRU cache when entry from...

2018-09-25 Thread manishgupta88 
GitHub user manishgupta88 opened a pull request:

https://github.com/apache/carbondata/pull/2759

[HOTFIX] Fix NPE in LRU cache when entry from the same table is getting 
evicted to load another entry from same table

**Problem**
When driver LRU cache size is configured to a small value then on running 
concurrent queries sometimes while loading the block dataMap in LRU cache one 
of the dataMap entries from the same table is getting deleted because of 
shortage of space. Due to this in the flow after loading the dataMap cache NPE 
is thrown.
This is because when an cacheable entry is removed from LRU cache then 
invalidate is called on that cacheable entry to clear the unsafe memory used by 
that entry. Invalidate method makes the references null and clears the unsafe 
memory which leads to NPE when accessed again.

**Solution**
Currently dataMap cache uses unsafe offheap memory for datamap caching. To 
avoid this the code is modified to use unsafe with onheap so that JVM itself 
takes care of clearing the memory when required. We do not require to 
explicitly set the references to null.

 - [ ] Any interfaces changed?
 No
 - [ ] Any backward compatibility impacted?
 No
 - [ ] Document update required?
No
 - [ ] Testing done
Yes verified manually   
 - [ ] For large changes, please consider breaking it into sub-tasks under 
an umbrella JIRA. 
NA


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/manishgupta88/carbondata lru_cache_NPE

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/carbondata/pull/2759.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2759


commit 598fceb3e7ee4f5797ef75ef04cd3b50c426984b
Author: manishgupta88 
Date:   2018-09-25T13:51:08Z

Fix NPE in LRU cache when entry from the same table is gettign evicted to 
load another entry




---