From 8201176530e656b44be58c9b06acba19cf8a04cc Mon Sep 17 00:00:00 2001 From: Amit Patel Date: Mon, 14 Aug 2017 15:27:40 -0700 Subject: [PATCH] HBASE-18508 Potential fix for timed out tests by resetting EEM in TestIncrementTimeRange and TestCellACLWithMultipleVersions. Injecting an edge into an EEM and then attempting to start a minicluster will cause time out. These tests do not reset the edge so it is likely they are causing the other tests to time out. Changes: - Properly map timestamps in AccessController#checkCoveringPermission - Setting of time ranges in TestCellACLWithMultipleVersions now uses epoch time instead of timestamp time --- .../org/apache/hadoop/hbase/regionserver/HRegion.java | 15 +++++---------- .../hadoop/hbase/security/access/AccessController.java | 14 ++++++++++++-- .../hadoop/hbase/coprocessor/TestIncrementTimeRange.java | 1 + .../security/access/TestCellACLWithMultipleVersions.java | 8 ++++---- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 59de679b59..e81949cec6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -2842,26 +2842,21 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi * Clients use physical timestamps when setting time ranges. Tables that use HLCs must map the * physical timestamp to HLC time */ - private void mapTimeRangesWithRespectToClock(Scan scan) { - TimeRange tr = scan.getTimeRange(); + public TimeRange mapTimeRangesWithRespectToClock(TimeRange tr) { if (tr.isAllTime()) { - return; + return tr; } TimestampType timestampType = getClock().getTimestampType(); // Clip time range max to prevent overflow when converting from epoch time to timestamp time long trMaxClipped = Math.min(tr.getMax(), timestampType.getMaxPhysicalTime()); - try { - scan.setTimeRange(timestampType.fromEpochTimeMillisToTimestamp(tr.getMin()), - timestampType.fromEpochTimeMillisToTimestamp(trMaxClipped)); - } catch (IOException e) { - e.printStackTrace(); - } + return new TimeRange(timestampType.fromEpochTimeMillisToTimestamp(tr.getMin()), + timestampType.fromEpochTimeMillisToTimestamp(trMaxClipped)); } private RegionScanner getScanner(Scan scan, List additionalScanners, long nonceGroup, long nonce) throws IOException { if (getClock().getClockType() == ClockType.HYBRID_LOGICAL) { - mapTimeRangesWithRespectToClock(scan); + scan.setTimeRange(mapTimeRangesWithRespectToClock(scan.getTimeRange())); } startRegionOperation(Operation.SCAN); try { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java index 429180d50e..a9f634448c 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java @@ -39,6 +39,7 @@ import org.apache.hadoop.hbase.ArrayBackedTag; import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellScanner; import org.apache.hadoop.hbase.CellUtil; +import org.apache.hadoop.hbase.ClockType; import org.apache.hadoop.hbase.CompoundConfiguration; import org.apache.hadoop.hbase.CoprocessorEnvironment; import org.apache.hadoop.hbase.DoNotRetryIOException; @@ -96,6 +97,7 @@ import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.AccessControlService; +import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.regionserver.InternalScanner; import org.apache.hadoop.hbase.regionserver.MiniBatchOperationInProgress; import org.apache.hadoop.hbase.regionserver.Region; @@ -776,17 +778,25 @@ public class AccessController implements MasterObserver, RegionObserver, RegionS // the Get to the current server time. We add 1 to the timerange since // the upper bound of a timerange is exclusive yet we need to examine // any cells found there inclusively. + // Convert time range timestamp time to compare against cell timestamp + if (opTs != HConstants.LATEST_TIMESTAMP) { + opTs = regionEnv.getRegion().getClock().getTimestampType().fromEpochTimeMillisToTimestamp(opTs); + } long latestTs = Math.max(opTs, latestCellTs); if (latestTs == 0 || latestTs == HConstants.LATEST_TIMESTAMP) { if (latestCellTs == HConstants.LATEST_TIMESTAMP || latestCellTs == 0) { latestTs = HConstants.LATEST_TIMESTAMP - 1; - } else if (TimestampType.HYBRID.isLikelyOfType(latestCellTs)) { + } else if (regionEnv.getRegion().getClock().getTimestampType() == TimestampType.HYBRID) { latestTs = TimestampType.HYBRID.fromEpochTimeMillisToTimestamp(EnvironmentEdgeManager - .currentTime()); + .currentTime()); } else { latestTs = EnvironmentEdgeManager.currentTime(); } } + // Convert back to epoch time for time range in get + if (latestTs < HConstants.LATEST_TIMESTAMP - 1) { + latestTs = regionEnv.getRegion().getClock().getTimestampType().toEpochTimeMillisFromTimestamp(latestTs); + } get.setTimeRange(0, latestTs + 1); // In case of Put operation we set to read all versions. This was done to consider the case // where columns are added with TS other than the Mutation TS. But normally this wont be the diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestIncrementTimeRange.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestIncrementTimeRange.java index ed30a2d5b0..229b238596 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestIncrementTimeRange.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestIncrementTimeRange.java @@ -117,6 +117,7 @@ public class TestIncrementTimeRange { @AfterClass public static void tearDownAfterClass() throws Exception { + EnvironmentEdgeManager.reset(); util.shutdownMiniCluster(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLWithMultipleVersions.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLWithMultipleVersions.java index 7cabda11f0..e99b49c72c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLWithMultipleVersions.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCellACLWithMultipleVersions.java @@ -522,6 +522,7 @@ public class TestCellACLWithMultipleVersions extends SecureTestUtil { try (Table t = connection.getTable(TEST_TABLE.getTableName())) { // This version (TS = 123) with rw ACL for USER_OTHER and USER_OTHER2 Put p = new Put(TEST_ROW); + now = EnvironmentEdgeManager.currentTime(); EnvironmentEdgeManager.injectEdge(mee); now += 123; mee.setValue(now); @@ -767,8 +768,7 @@ public class TestCellACLWithMultipleVersions extends SecureTestUtil { try (Connection connection = ConnectionFactory.createConnection(conf)) { try (Table t = connection.getTable(TEST_TABLE.getTableName())) { Increment inc = new Increment(TEST_ROW1); - inc.setTimeRange(0, timestampType.toTimestamp(TimeUnit.MILLISECONDS, now, - timestampType.getMaxLogicalTime())); + inc.setTimeRange(0, now + 1); inc.addColumn(TEST_FAMILY1, TEST_Q1, 2L); t.increment(inc); t.incrementColumnValue(TEST_ROW1, TEST_FAMILY1, TEST_Q2, 1L); @@ -790,8 +790,7 @@ public class TestCellACLWithMultipleVersions extends SecureTestUtil { try (Connection connection = ConnectionFactory.createConnection(conf)) { try (Table t = connection.getTable(TEST_TABLE.getTableName())) { Increment inc = new Increment(row); - inc.setTimeRange(0, timestampType.toTimestamp(TimeUnit.MILLISECONDS, now+4, - timestampType.getMaxLogicalTime())); + inc.setTimeRange(0, now + 5); inc.addColumn(TEST_FAMILY1, q1, 2L); t.increment(inc); fail(user.getShortName() + " cannot do the increment."); @@ -1052,6 +1051,7 @@ public class TestCellACLWithMultipleVersions extends SecureTestUtil { @After public void tearDown() throws Exception { + EnvironmentEdgeManager.reset(); // Clean the _acl_ table try { TEST_UTIL.deleteTable(TEST_TABLE.getTableName()); -- 2.11.0 (Apple Git-81)