[GitHub] [hbase] apurtell commented on pull request #3234: HBASE-25854 Remove redundant AM in-memory state changes in CatalogJanitor
apurtell commented on pull request #3234: URL: https://github.com/apache/hbase/pull/3234#issuecomment-833220235 `TestReplicationSource` failure is not related. [INFO] --- [INFO] T E S T S [INFO] --- [INFO] Running org.apache.hadoop.hbase.replication.regionserver.TestReplicationSource [INFO] Tests run: 12, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 52.258 s - in org.apache.hadoop.hbase.replication.regionserver.TestReplicationSource [INFO] [INFO] Results: [INFO] Tests run: 12, Failures: 0, Errors: 0, Skipped: 0 Actual failing test `TestCatalogJanitorInMemoryStates` was fixed with e7db51f . -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] apurtell commented on pull request #3234: HBASE-25854 Remove redundant AM in-memory state changes in CatalogJanitor
apurtell commented on pull request #3234: URL: https://github.com/apache/hbase/pull/3234#issuecomment-833128705 Ok, let me rebase this and fix the unit tests. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] apurtell commented on pull request #3234: HBASE-25854 Remove redundant AM in-memory state changes in CatalogJanitor
apurtell commented on pull request #3234: URL: https://github.com/apache/hbase/pull/3234#issuecomment-833127897 > (I just noticed the wonky split message -- have started a bit of ITBLL over here...) I've committed a few changes over the past couple of days. One was an accumulation of SPLIT state regionStates for as long as CatalogJanitor is deferring cleanup, because daughters still have references, because compaction is backed up. Those caused concerning warnings at each balancer iteration. Another was a race condition between split handling and regionserver report handling that could cause multiple split procedures to get scheduled for the same split request. Only one could succeed. The others would cause log noise but their failures were harmless. Would be good to get a second opinion. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] apurtell commented on pull request #3234: HBASE-25854 Remove redundant AM in-memory state changes in CatalogJanitor
apurtell commented on pull request #3234: URL: https://github.com/apache/hbase/pull/3234#issuecomment-832857609 Although the unit test will fail, when I tried this change out on a cluster in a very write heavy ingestion test, the end result was good. The ingestion test completes successfully and without weird artifacts in the logging. All split regions are GCed by procedures. In memory state aligns with filesystem state. /hbase/logs/hbase-apurtell-master-ip-172-31-58-47.us-west-2.compute.internal.log:130972:2021-05-05 15:47:39,190 INFO [master/ip-172-31-58-47:8100.Chore.1] master.HbckChore: Loaded 230 regions (0 disabled, 0 split parents) from in-memory state /hbase/logs/hbase-apurtell-master-ip-172-31-58-47.us-west-2.compute.internal.log:130973:2021-05-05 15:47:39,190 DEBUG [master/ip-172-31-58-47:8100.Chore.1] master.HbckChore: Regions by state: OPEN=230 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] apurtell commented on pull request #3234: HBASE-25854 Remove redundant AM in-memory state changes in CatalogJanitor
apurtell commented on pull request #3234: URL: https://github.com/apache/hbase/pull/3234#issuecomment-832855720 @Apache9 added some context on https://github.com/apache/hbase/pull/3230 > Some background > > https://issues.apache.org/jira/browse/HBASE-24942 > > I've found a bug when refactoring CatalogJanitor because of we may clean the merge qualifiers in MergeTableRegionsProcedure. Theoretically we should only do this in GCRegion related procedures(as what you proposed here), but doing this in other places could speed up later process which could be blocked by merge qualifiers. > > For me I'm +1 on removing the redundant removal in CatalogJanitor, but let's wait for @saintstack 's opinon too? > > Thanks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [hbase] apurtell commented on pull request #3234: HBASE-25854 Remove redundant AM in-memory state changes in CatalogJanitor
apurtell commented on pull request #3234: URL: https://github.com/apache/hbase/pull/3234#issuecomment-832854650 This change is moved here from https://github.com/apache/hbase/pull/3230. We know from the test report there that `TestCatalogJanitorInMemoryStates` will fail. Let's let it fail here too and then proceed. java.lang.AssertionError: Parent region should have been removed from RegionStates at org.apache.hadoop.hbase.master.janitor.TestCatalogJanitorInMemoryStates.testInMemoryParentCleanup(TestCatalogJanitorInMemoryStates.java:123) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
