Details
-
Sub-task
-
Status: Closed
-
Major
-
Resolution: Fixed
-
None
-
None
-
Reviewed
Description
With offheap MSLAB in place we may have to change the flush related heuristics to work with offheap size configured rather than the java heap size.
Since we now have clear seperation of the memstore data size and memstore heap size, for offheap memstore
-> Decide if the global.offheap.memstore.size is breached for blocking updates and force flushes.
-> If the onheap global.memstore.size is breached (due to heap overhead) even then block updates and force flushes.
-> The global.memstore.size.lower.limit is now by default 95% of the global.memstore.size. So now we apply this 95% on the global.offheap.memstore.size and also on global.memstore.size (as it was done for onheap case).
-> We will have new FlushTypes introduced
ABOVE_ONHEAP_LOWER_MARK, /* happens due to lower mark breach of onheap memstore settings
An offheap memstore can even breach the onheap_lower_mark*/
ABOVE_ONHEAP_HIGHER_MARK,/* happens due to higher mark breach of onheap memstore settings
An offheap memstore can even breach the onheap_higher_mark*/
ABOVE_OFFHEAP_LOWER_MARK,/* happens due to lower mark breach of offheap memstore settings*/
ABOVE_OFFHEAP_HIGHER_MARK;
-> regionServerAccounting does all the accounting.
-> HeapMemoryTuner is what is litte tricky here. First thing to note is that at no point it will try to increase or decrease the global.offheap.memstore.size. If there is a heap pressure then it will try to increase the memstore heap limit.
In case of offheap memstore there is always a chance that the heap pressure does not increase. In that case we could ideally decrease the heap limit for memstore. The current logic of heapmemory tuner is such that things will naturally settle down. But on discussion what we thought is let us include the flush count that happens due to offheap pressure but give that a lesser weightage and thus ensure that the initial decrease on memstore heap limit does not happen. Currently that fraction is set as 0.5.
Attachments
Attachments
- HBASE-15787_1.patch
- 73 kB
- ramkrishna.s.vasudevan
- HBASE-15787_4.patch
- 79 kB
- ramkrishna.s.vasudevan
- HBASE-15787_5.patch
- 81 kB
- ramkrishna.s.vasudevan
- HBASE-15787_6.patch
- 80 kB
- ramkrishna.s.vasudevan
- HBASE-15787_7.patch
- 79 kB
- ramkrishna.s.vasudevan
- HBASE-15787_8.patch
- 79 kB
- ramkrishna.s.vasudevan
- HBASE-15787_9.patch
- 80 kB
- ramkrishna.s.vasudevan
- HBASE-15787.patch
- 59 kB
- ramkrishna.s.vasudevan
Issue Links
- links to
Activity
Am not able to add this to RB unless HBASE-15786 is completed I believe. Will work on adding test cases.
this patch adds test case and does have better mechanism to track offheap memstore affecting the heap memory manager.
If we have offheap memstore and we are sure that there are no flushes due to onheap pressure, the 'step' function that we apply to decrease the memstore size incase we need to increase the block_cache size (under read heavy cases) we do that with minstep function. this wil ensure that we don't reduce the heap limit of the offheap memstore which will lead to heap overhead flushes.
In cases where there are only flushes due to offheap pressure and it is write scenario we won't adjust the memstore size at all. All cases covered in test cases.
Pls have a look at it.
DefaultHeapMemoryTuner dealing with off heap flush count etc.. IMO we should avoid this.. Got it why we are doing so.. In case of off heap MSLAB, the forced flushes might be because of global off heap memstore size pressure or by global heap memstore overhead. When it is because of the latter, we need to do some thing in the Heap memory tuner (If possible) Any way we pass this as blocked flush count. So when we have the flushes because of heap memory overhead pressure, we account that any way so as to get some help from the tuner. (possible up)) But when there are no pressure from heap size, and have lack of space in L1 cache, tuner should down the memstore % and give that space for L1 cache.. That is ur intent.. I agree to this. And any way we need to consider flushes because of off heap pressure also.. After all these counts are used to check whether we have R/W workload and what extend.
May be we can do some sort of math and pass blocked flushes, unblocked flush with out separation of heap/off heap
Blocked flush = blocked flush by heap overhead pressure + 0.75 * blocked flush by off heap pressure
Same with unblocked flush count
Why we need on heap off heap diff types in FlushType ?
DefaultHeapMemoryTuner dealing with off heap flush count etc.. IMO we should avoid this.
I think here we are not dealing or tuning the offheap memory. What we are doing is that we are using those counts to make a better onheap tuning decision. So I don't think we are making the heap memory tuner to deal with offheap. That is what I think.
Why we need on heap off heap diff types in FlushType ?
I just need that to know what is the exact flush count I get even when we have onheap memstore or offheap memstore. May be this enum we can move out of the tuner? I think I just added some comments over there in the patch to decide on it.
Actually the decision to increase/decrease memstore size and block size is more intelligent now. It does in terms terms of smaller steps. Say if we have global memstore limit as 12G even if we decrease the fraction by 0.1 (say from 0.42 to 0.32 and xmx is 30G) we drastically go down to 9G. But the current code does not do that way. It still tries to do it with smaller steps.
So when there is an offheap memstore and the reads are more and we need more block cache space we need to reduce heap memstore limit. In that case we just try to reduce with the minimum step size.
It is not directly possible to apply something like this
Blocked flush = blocked flush by heap overhead pressure + 0.75 * blocked flush by off heap pressure
We can only indirectly change the step size and in this case we try to make the step size as less as possible.
One thing is that in case of R/W workload eventually it will settle down in course of time or as in the current impl if we are not able to take a decision the current setting are left intact. So I think by all means we are safe.
One note is that we should allow specifying less than full percentages and/or finder granularity decimals. 1% of 100G is 1G which is a lot of RAM. Can we add being able to specify to two decimal places in percntages and four if decimal at least doing heap calculations?
Is this up on rb?
I will put this up on RB. Actually the heap memory tuner is smart enough such that it is not doing reduction 1% at a time instead it does in steps. The step starts from 0.00125f upto 0.04f. So this ensures that any reduction or increase is not drastic.
So in this patch if there is offheap pressure flushes alone and there is a need to increase the block cache size - we try to reduce the heap memstore size using the minimum step size (0.00125).
And also the heap tuner is also smart enough to decide if it needs to be neutral if the inputs that it receives does not allow it to decide what to do and eventually things settle down. Thank you.
Looking at the patch, can DefaultHeapMemoryTuner be made standalone so it can be tested in isolation? I'd think getting initial values right would be easy enough but would be cool if we could run a scenario and watch how the feedback loop develops over a period of time... see if it settles or if it does a Tacoma Narrows Bridge behavior.
RegionServerAccounting is memory only? Should it say that it is memory only? RegionServerMemoryAccounting? Or just MemoryAccounting since the RegionServer is a given?
This seems pretty important given as we don't have a good way at looking at offheap:
// TODO : add support for offheap metrics
Why am I doing ABOVE_OFFHEAP_HIGHER_MARK in the HeapMemoryManager (unless HeapMemoryManager is for both onheap and offheap?)
I gave it a pass.
ABOVE_OFFHEAP_HIGHER_MARK - I need this to decide what caused the flush. I don't allow the heap tuner to tune the offheap but only uses this metric to know if I am allowed to reduce the memstore heap size.
I will try to remove this as part of my TODO
// See if we really want this?? Will change or leave it + // after discussion
// TODO : add support for offheap metrics
Metric I will add a new JIRA to cover offheap things. So that is why there is a TODO. Will post in RB after adding some comments.
Why am I doing ABOVE_OFFHEAP_HIGHER_MARK in the HeapMemoryManager (unless HeapMemoryManager is for both onheap and offheap?)
Am removing this for now. Except that am seeing if this heap memory tuner is called for offheap memstore. If so I do some step size calculations.
see if it settles or if it does a Tacoma Narrows Bridge behavior.
I first read what is this behaviour . Interesting. And true that we should be careful that we don't end up there. But for that I think we need to run various work loads. So will that be good if we commit this and as part of testing we see what test cases we need to consider before making this auto tuner default for 2.0?
RegionServerAccounting is memory only? Should it say that it is memory only?
I think the purpose was not only memory.
/** * RegionServerAccounting keeps record of some basic real time information about * the Region Server.
The javadoc says basic real time info which includes memory. So am not sure if it is right to rename to MemoryAccounting. If you strongly feel about the rename I can do it.
New patch. Remove OFFHEAP_HIGH(LOW)_MARK from Heapmemory tuner. Except that it knows whether the current memstore is offheap memstore.
Will add to RB.
Ping saint.ack@gmail.com.
What do you think - how to go further with this. Can you see the current patch in RB?
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 18s | Docker mode activated. |
+1 | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 4 new or modified test files. |
+1 | mvninstall | 2m 49s | master passed |
+1 | compile | 0m 35s | master passed |
+1 | checkstyle | 0m 44s | master passed |
+1 | mvneclipse | 0m 13s | master passed |
+1 | findbugs | 1m 42s | master passed |
+1 | javadoc | 0m 27s | master passed |
+1 | mvninstall | 0m 41s | the patch passed |
+1 | compile | 0m 35s | the patch passed |
+1 | javac | 0m 35s | the patch passed |
+1 | checkstyle | 0m 44s | the patch passed |
+1 | mvneclipse | 0m 13s | the patch passed |
-1 | whitespace | 0m 0s | The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix. |
+1 | hadoopcheck | 25m 35s | Patch does not cause any errors with Hadoop 2.6.1 2.6.2 2.6.3 2.6.4 2.6.5 2.7.1 2.7.2 2.7.3 or 3.0.0-alpha1. |
-1 | findbugs | 1m 57s | hbase-server generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
-1 | javadoc | 0m 26s | hbase-server generated 1 new + 1 unchanged - 0 fixed = 2 total (was 1) |
-1 | unit | 14m 12s | hbase-server in the patch failed. |
+1 | asflicense | 0m 8s | The patch does not generate ASF License warnings. |
51m 40s |
Reason | Tests |
---|---|
FindBugs | module:hbase-server |
int value cast to float and then passed to Math.round in org.apache.hadoop.hbase.regionserver.wal.AbstractFSWAL.calculateMaxLogFiles(Configuration, long) At AbstractFSWAL.java:and then passed to Math.round in org.apache.hadoop.hbase.regionserver.wal.AbstractFSWAL.calculateMaxLogFiles(Configuration, long) At AbstractFSWAL.java:[line 289] | |
Failed junit tests | hadoop.hbase.regionserver.TestHeapMemoryManager |
This message was automatically generated.
+1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 22s | Docker mode activated. |
+1 | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 4 new or modified test files. |
+1 | mvninstall | 3m 11s | master passed |
+1 | compile | 0m 35s | master passed |
+1 | checkstyle | 0m 44s | master passed |
+1 | mvneclipse | 0m 13s | master passed |
+1 | findbugs | 1m 42s | master passed |
+1 | javadoc | 0m 27s | master passed |
+1 | mvninstall | 0m 39s | the patch passed |
+1 | compile | 0m 35s | the patch passed |
+1 | javac | 0m 35s | the patch passed |
+1 | checkstyle | 0m 44s | the patch passed |
+1 | mvneclipse | 0m 13s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | hadoopcheck | 25m 41s | Patch does not cause any errors with Hadoop 2.6.1 2.6.2 2.6.3 2.6.4 2.6.5 2.7.1 2.7.2 2.7.3 or 3.0.0-alpha1. |
+1 | findbugs | 1m 50s | the patch passed |
+1 | javadoc | 0m 26s | the patch passed |
+1 | unit | 89m 24s | hbase-server in the patch passed. |
+1 | asflicense | 0m 17s | The patch does not generate ASF License warnings. |
127m 21s |
Subsystem | Report/Notes |
---|---|
Docker | Client=1.11.2 Server=1.11.2 Image:yetus/hbase:8d52d23 |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12842099/HBASE-15787_6.patch |
JIRA Issue | |
Optional Tests | asflicense javac javadoc unit findbugs hadoopcheck hbaseanti checkstyle compile |
uname | Linux 3a8c611b0030 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh |
git revision | master / 1eb24e4 |
Default Java | 1.8.0_111 |
findbugs | v3.0.0 |
Test Results | https://builds.apache.org/job/PreCommit-HBASE-Build/4825/testReport/ |
modules | C: hbase-server U: hbase-server |
Console output | https://builds.apache.org/job/PreCommit-HBASE-Build/4825/console |
Powered by | Apache Yetus 0.3.0 http://yetus.apache.org |
This message was automatically generated.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 17s | Docker mode activated. |
+1 | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 4 new or modified test files. |
+1 | mvninstall | 2m 53s | master passed |
+1 | compile | 0m 35s | master passed |
+1 | checkstyle | 0m 42s | master passed |
+1 | mvneclipse | 0m 12s | master passed |
+1 | findbugs | 1m 40s | master passed |
+1 | javadoc | 0m 27s | master passed |
-1 | mvninstall | 0m 35s | hbase-server in the patch failed. |
-1 | compile | 0m 35s | hbase-server in the patch failed. |
-1 | javac | 0m 35s | hbase-server in the patch failed. |
+1 | checkstyle | 0m 41s | the patch passed |
+1 | mvneclipse | 0m 12s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
-1 | hadoopcheck | 1m 14s | The patch causes 14 errors with Hadoop v2.6.1. |
-1 | hadoopcheck | 2m 27s | The patch causes 14 errors with Hadoop v2.6.2. |
-1 | hadoopcheck | 3m 36s | The patch causes 14 errors with Hadoop v2.6.3. |
-1 | hadoopcheck | 4m 45s | The patch causes 14 errors with Hadoop v2.6.4. |
-1 | hadoopcheck | 5m 54s | The patch causes 14 errors with Hadoop v2.6.5. |
-1 | hadoopcheck | 7m 3s | The patch causes 14 errors with Hadoop v2.7.1. |
-1 | hadoopcheck | 8m 10s | The patch causes 14 errors with Hadoop v2.7.2. |
-1 | hadoopcheck | 9m 20s | The patch causes 14 errors with Hadoop v2.7.3. |
-1 | hadoopcheck | 10m 27s | The patch causes 14 errors with Hadoop v3.0.0-alpha1. |
-1 | findbugs | 0m 19s | hbase-server in the patch failed. |
+1 | javadoc | 0m 25s | the patch passed |
-1 | unit | 0m 34s | hbase-server in the patch failed. |
+1 | asflicense | 0m 8s | The patch does not generate ASF License warnings. |
21m 0s |
This message was automatically generated.
+1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 11s | Docker mode activated. |
+1 | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 4 new or modified test files. |
+1 | mvninstall | 2m 54s | master passed |
+1 | compile | 0m 35s | master passed |
+1 | checkstyle | 0m 41s | master passed |
+1 | mvneclipse | 0m 13s | master passed |
+1 | findbugs | 1m 42s | master passed |
+1 | javadoc | 0m 26s | master passed |
+1 | mvninstall | 0m 41s | the patch passed |
+1 | compile | 0m 37s | the patch passed |
+1 | javac | 0m 37s | the patch passed |
+1 | checkstyle | 0m 43s | the patch passed |
+1 | mvneclipse | 0m 14s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | hadoopcheck | 25m 34s | Patch does not cause any errors with Hadoop 2.6.1 2.6.2 2.6.3 2.6.4 2.6.5 2.7.1 2.7.2 2.7.3 or 3.0.0-alpha1. |
+1 | findbugs | 1m 49s | the patch passed |
+1 | javadoc | 0m 24s | the patch passed |
+1 | unit | 88m 52s | hbase-server in the patch passed. |
+1 | asflicense | 0m 14s | The patch does not generate ASF License warnings. |
126m 10s |
Subsystem | Report/Notes |
---|---|
Docker | Client=1.12.3 Server=1.12.3 Image:yetus/hbase:8d52d23 |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12843197/HBASE-15787_8.patch |
JIRA Issue | |
Optional Tests | asflicense javac javadoc unit findbugs hadoopcheck hbaseanti checkstyle compile |
uname | Linux 7267434fa456 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build@2/component/dev-support/hbase-personality.sh |
git revision | master / a73b0b3 |
Default Java | 1.8.0_111 |
findbugs | v3.0.0 |
Test Results | https://builds.apache.org/job/PreCommit-HBASE-Build/4914/testReport/ |
modules | C: hbase-server U: hbase-server |
Console output | https://builds.apache.org/job/PreCommit-HBASE-Build/4914/console |
Powered by | Apache Yetus 0.3.0 http://yetus.apache.org |
This message was automatically generated.
This is what I will be committing. The getFlushPressure() is added in RegionserverAccounting.
We could do a clean up of the API in RegionServerServices as a separate task.
+1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 9s | Docker mode activated. |
+1 | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
+1 | test4tests | 0m 0s | The patch appears to include 4 new or modified test files. |
+1 | mvninstall | 3m 3s | master passed |
+1 | compile | 0m 36s | master passed |
+1 | checkstyle | 0m 45s | master passed |
+1 | mvneclipse | 0m 13s | master passed |
+1 | findbugs | 1m 47s | master passed |
+1 | javadoc | 0m 27s | master passed |
+1 | mvninstall | 0m 42s | the patch passed |
+1 | compile | 0m 36s | the patch passed |
+1 | javac | 0m 36s | the patch passed |
+1 | checkstyle | 0m 44s | the patch passed |
+1 | mvneclipse | 0m 13s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | hadoopcheck | 25m 39s | Patch does not cause any errors with Hadoop 2.6.1 2.6.2 2.6.3 2.6.4 2.6.5 2.7.1 2.7.2 2.7.3 or 3.0.0-alpha1. |
+1 | findbugs | 1m 54s | the patch passed |
+1 | javadoc | 0m 27s | the patch passed |
+1 | unit | 90m 27s | hbase-server in the patch passed. |
+1 | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
128m 14s |
Subsystem | Report/Notes |
---|---|
Docker | Client=1.12.3 Server=1.12.3 Image:yetus/hbase:8d52d23 |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12843400/HBASE-15787_9.patch |
JIRA Issue | |
Optional Tests | asflicense javac javadoc unit findbugs hadoopcheck hbaseanti checkstyle compile |
uname | Linux 91481bf6a4f4 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh |
git revision | master / f9750f2 |
Default Java | 1.8.0_111 |
findbugs | v3.0.0 |
Test Results | https://builds.apache.org/job/PreCommit-HBASE-Build/4930/testReport/ |
modules | C: hbase-server U: hbase-server |
Console output | https://builds.apache.org/job/PreCommit-HBASE-Build/4930/console |
Powered by | Apache Yetus 0.3.0 http://yetus.apache.org |
This message was automatically generated.
Seems QA is clean. Can I get a +1 on this anoop.hbase so that will go for the commit.
Thanks for the reviews saint.ack@gmail.com and anoop.hbase.
Pushed to master.
FAILURE: Integrated in Jenkins build HBase-Trunk_matrix #2137 (See https://builds.apache.org/job/HBase-Trunk_matrix/2137/)
HBASE-15787 Change the flush related heuristics to work with offheap (ramkrishna: rev d1147eeb7e1d5f41161c7cf5bc5ddb4744ca5b57)
- (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultHeapMemoryTuner.java
- (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java
- (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java
- (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreChunkPool.java
- (edit) hbase-server/src/test/java/org/apache/hadoop/hbase/TestGlobalMemStoreSize.java
- (add) hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerAccounting.java
- (edit) hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionReplayEvents.java
- (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreFlusher.java
- (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsHeapMemoryManager.java
- (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/io/util/MemorySizeUtil.java
- (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerAccounting.java
- (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
- (edit) hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHeapMemoryManager.java
- (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java
This patch is built on top of
HBASE-15786. There are no tests added for now. Will add in subsequent patches. Will update the description.