From 3045d42305fcdfeeb13a8e4894b4354b6291a65b Mon Sep 17 00:00:00 2001 From: Francis Liu Date: Thu, 7 Jun 2018 10:24:28 -0700 Subject: [PATCH] HBASE-20704 Sometimes compacted storefiles are not archived on region close --- .../apache/hadoop/hbase/regionserver/HStore.java | 30 ++++- .../TestCleanupCompactedFileOnRegionClose.java | 142 +++++++++++++++++++++ 2 files changed, 166 insertions(+), 6 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCleanupCompactedFileOnRegionClose.java diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java index 9494e18dad..08b8738044 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java @@ -926,7 +926,7 @@ public class HStore implements Store, HeapSize, StoreConfigInformation, Propagat storeEngine.getStoreFileManager().clearCompactedFiles(); // clear the compacted files if (CollectionUtils.isNotEmpty(compactedfiles)) { - removeCompactedfiles(compactedfiles); + removeCompactedfiles(compactedfiles, true); } if (!result.isEmpty()) { // initialize the thread pool for closing store files in parallel. @@ -2565,7 +2565,7 @@ public class HStore implements Store, HeapSize, StoreConfigInformation, Propagat lock.readLock().unlock(); } if (CollectionUtils.isNotEmpty(copyCompactedfiles)) { - removeCompactedfiles(copyCompactedfiles); + removeCompactedfiles(copyCompactedfiles, false); } } finally { archiveLock.unlock(); @@ -2576,7 +2576,7 @@ public class HStore implements Store, HeapSize, StoreConfigInformation, Propagat * Archives and removes the compacted files * @param compactedfiles The compacted files in this store that are not active in reads */ - private void removeCompactedfiles(Collection compactedfiles) + private void removeCompactedfiles(Collection compactedfiles, boolean storeClosing) throws IOException { final List filesToRemove = new ArrayList<>(compactedfiles.size()); final List storeFileSizes = new ArrayList<>(compactedfiles.size()); @@ -2594,7 +2594,21 @@ public class HStore implements Store, HeapSize, StoreConfigInformation, Propagat storeFileSizes.add(length); continue; } - if (file.isCompactedAway() && !file.isReferencedInReads()) { + + //Compacted files in the list should always be marked compacted away. In the event + //they're contradicting in order to guarantee data consistency + //should we choose one and ignore the other? + if (storeClosing && !file.isCompactedAway()) { + LOG.warn("Region closing but StoreFile is in compacted list but not compacted away: {}", + file.getPath().getName()); + } + + //If store is closing we're ignoring any references to keep things consistent + //and remove compacted storefiles from the region directory + if (file.isCompactedAway() && (!file.isReferencedInReads() || storeClosing)) { + if (storeClosing && file.isReferencedInReads()) { + LOG.debug("Region closing but StoreFile still has references: {}", file.getPath().getName()); + } // Even if deleting fails we need not bother as any new scanners won't be // able to use the compacted file as the status is already compactedAway LOG.trace("Closing and archiving the file {}", file); @@ -2607,8 +2621,12 @@ public class HStore implements Store, HeapSize, StoreConfigInformation, Propagat storeFileSizes.add(length); } } catch (Exception e) { - LOG.error("Exception while trying to close the compacted store file {}", - file.getPath(), e); + String msg = "Exception while trying to close the compacted store file " + file.getPath().getName(); + LOG.error(msg, e); + //if we get an exception let caller know so it can abort the server + if (storeClosing) { + throw new IOException(msg, e); + } } } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCleanupCompactedFileOnRegionClose.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCleanupCompactedFileOnRegionClose.java new file mode 100644 index 0000000000..c24e6e1795 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCleanupCompactedFileOnRegionClose.java @@ -0,0 +1,142 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.regionserver; + +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Delete; +import org.apache.hadoop.hbase.client.Get; +import org.apache.hadoop.hbase.client.HBaseAdmin; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.Result; +import org.apache.hadoop.hbase.client.ResultScanner; +import org.apache.hadoop.hbase.client.Scan; +import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.regionserver.compactions.CompactionConfiguration; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import java.util.Collection; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +@Category({MediumTests.class}) +public class TestCleanupCompactedFileOnRegionClose { + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestCleanupCompactedFileOnRegionClose.class); + + private static HBaseTestingUtility util; + + @BeforeClass + public static void beforeClass() throws Exception { + util = new HBaseTestingUtility(); + util.getConfiguration().setInt(CompactionConfiguration.HBASE_HSTORE_COMPACTION_MIN_KEY,100); + util.startMiniCluster(2); + } + + @AfterClass + public static void afterclass() throws Exception { + util.shutdownMiniCluster(); + } + + @Test + public void test() throws Exception { + TableName tableName = TableName.valueOf("foo"); + String familyName = "f"; + byte[] familyNameBytes = Bytes.toBytes(familyName); + util.createTable(tableName, familyName); + + HBaseAdmin hBaseAdmin = util.getHBaseAdmin(); + Table table = util.getConnection().getTable(tableName); + + HRegionServer rs = util.getRSForFirstRegionInTable(tableName); + Region region = rs.getRegions(tableName).get(0); + + int refSFCount = 4; + for (int i = 0; i < refSFCount; i++) { + for (int j = 0; j < refSFCount; j++) { + Put put = new Put(Bytes.toBytes(j)); + put.addColumn(familyNameBytes, Bytes.toBytes(i), Bytes.toBytes(j)); + table.put(put); + } + util.flush(tableName); + } + assertEquals(refSFCount, region.getStoreFileList(new byte[][]{familyNameBytes}).size()); + + //add a delete, to test wether we end up with an inconsistency post region close + Delete delete = new Delete(Bytes.toBytes(refSFCount-1)); + table.delete(delete); + util.flush(tableName); + assertFalse(table.exists(new Get(Bytes.toBytes(refSFCount-1)))); + + //Create a scanner and keep it open to add references to StoreFileReaders + Scan scan = new Scan(); + scan.setStopRow(Bytes.toBytes(refSFCount-2)); + scan.setCaching(1); + ResultScanner scanner = table.getScanner(scan); + Result res = scanner.next(); + assertNotNull(res); + assertEquals(refSFCount, res.getFamilyMap(familyNameBytes).size()); + + + //Verify the references + int count = 0; + for (HStoreFile sf : (Collection)region.getStore(familyNameBytes).getStorefiles()) { + synchronized (sf) { + if (count < refSFCount) { + assertTrue(sf.isReferencedInReads()); + } else { + assertFalse(sf.isReferencedInReads()); + } + } + count++; + } + + //Major compact to produce compacted storefiles that need to be cleaned up + util.compact(tableName, true); + assertEquals(1, region.getStoreFileList(new byte[][]{familyNameBytes}).size()); + assertEquals(refSFCount+1, + ((HStore)region.getStore(familyNameBytes)).getStoreEngine().getStoreFileManager().getCompactedfiles().size()); + + //close then open the region to determine wether compacted storefiles get cleaned up on close + hBaseAdmin.unassign(region.getRegionInfo().getRegionName(), false); + hBaseAdmin.assign(region.getRegionInfo().getRegionName()); + util.waitUntilNoRegionsInTransition(10000); + + + assertFalse("Deleted row should not exist", table.exists(new Get(Bytes.toBytes(refSFCount-1)))); + + rs = util.getRSForFirstRegionInTable(tableName); + region = rs.getRegions(tableName).get(0); + assertEquals(1, region.getStoreFileList(new byte[][]{familyNameBytes}).size()); + assertEquals(0, + ((HStore)region.getStore(familyNameBytes)).getStoreEngine().getStoreFileManager().getCompactedfiles().size()); + } +} -- 2.14.3 (Apple Git-98)