http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/ActivePolicyEnforcement.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/ActivePolicyEnforcement.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/ActivePolicyEnforcement.java index a313fa1..c558b26 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/ActivePolicyEnforcement.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/ActivePolicyEnforcement.java @@ -17,6 +17,7 @@ package org.apache.hadoop.hbase.quotas; import java.util.Collections; +import java.util.HashMap; import java.util.Map; import java.util.Objects; @@ -28,7 +29,12 @@ import org.apache.hadoop.hbase.regionserver.RegionServerServices; /** * A class to ease dealing with tables that have and do not have violation policies - * being enforced in a uniform manner. Immutable. + * being enforced. This class is immutable, expect for {@code locallyCachedPolicies}. + * + * The {@code locallyCachedPolicies} are mutable given the current {@code activePolicies} + * and {@code snapshots}. It is expected that when a new instance of this class is + * instantiated, we also want to invalidate those previously cached policies (as they + * may now be invalidate if we received new quota usage information). */ @InterfaceAudience.Private @InterfaceStability.Evolving @@ -36,12 +42,23 @@ public class ActivePolicyEnforcement { private final Map<TableName,SpaceViolationPolicyEnforcement> activePolicies; private final Map<TableName,SpaceQuotaSnapshot> snapshots; private final RegionServerServices rss; + private final SpaceViolationPolicyEnforcementFactory factory; + private final Map<TableName,SpaceViolationPolicyEnforcement> locallyCachedPolicies; public ActivePolicyEnforcement(Map<TableName,SpaceViolationPolicyEnforcement> activePolicies, Map<TableName,SpaceQuotaSnapshot> snapshots, RegionServerServices rss) { + this(activePolicies, snapshots, rss, SpaceViolationPolicyEnforcementFactory.getInstance()); + } + + public ActivePolicyEnforcement(Map<TableName,SpaceViolationPolicyEnforcement> activePolicies, + Map<TableName,SpaceQuotaSnapshot> snapshots, RegionServerServices rss, + SpaceViolationPolicyEnforcementFactory factory) { this.activePolicies = activePolicies; this.snapshots = snapshots; this.rss = rss; + this.factory = factory; + // Mutable! + this.locallyCachedPolicies = new HashMap<>(); } /** @@ -65,16 +82,25 @@ public class ActivePolicyEnforcement { */ public SpaceViolationPolicyEnforcement getPolicyEnforcement(TableName tableName) { SpaceViolationPolicyEnforcement policy = activePolicies.get(Objects.requireNonNull(tableName)); - if (null == policy) { - synchronized (activePolicies) { - // If we've never seen a snapshot, assume no use, and infinite limit - SpaceQuotaSnapshot snapshot = snapshots.get(tableName); - if (null == snapshot) { - snapshot = SpaceQuotaSnapshot.getNoSuchSnapshot(); + if (policy == null) { + synchronized (locallyCachedPolicies) { + // When we don't have an policy enforcement for the table, there could be one of two cases: + // 1) The table has no quota defined + // 2) The table is not in violation of its quota + // In both of these cases, we want to make sure that access remains fast and we minimize + // object creation. We can accomplish this by locally caching policies instead of creating + // a new instance of the policy each time. + policy = locallyCachedPolicies.get(tableName); + // We have already created/cached the enforcement, use it again. `activePolicies` and + // `snapshots` are immutable, thus this policy is valid for the lifetime of `this`. + if (policy != null) { + return policy; } - // Create the default policy and cache it - return SpaceViolationPolicyEnforcementFactory.getInstance().createWithoutViolation( - rss, tableName, snapshot); + // Create a PolicyEnforcement for this table and snapshot. The snapshot may be null + // which is OK. + policy = factory.createWithoutViolation(rss, tableName, snapshots.get(tableName)); + // Cache the policy we created + locallyCachedPolicies.put(tableName, policy); } } return policy; @@ -87,6 +113,14 @@ public class ActivePolicyEnforcement { return Collections.unmodifiableMap(activePolicies); } + /** + * Returns an unmodifiable version of the policy enforcements that were cached because they are + * not in violation of their quota. + */ + Map<TableName,SpaceViolationPolicyEnforcement> getLocallyCachedPolicies() { + return Collections.unmodifiableMap(locallyCachedPolicies); + } + @Override public String toString() { return getClass().getSimpleName() + ": " + activePolicies;
http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/FileSystemUtilizationChore.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/FileSystemUtilizationChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/FileSystemUtilizationChore.java index efc17ff..418a163 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/FileSystemUtilizationChore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/FileSystemUtilizationChore.java @@ -81,7 +81,7 @@ public class FileSystemUtilizationChore extends ScheduledChore { Iterator<Region> oldRegionsToProcess = getLeftoverRegions(); final Iterator<Region> iterator; final boolean processingLeftovers; - if (null == oldRegionsToProcess) { + if (oldRegionsToProcess == null) { iterator = onlineRegions.iterator(); processingLeftovers = false; } else { @@ -179,6 +179,8 @@ public class FileSystemUtilizationChore extends ScheduledChore { * Reports the computed region sizes to the currently active Master. * * @param onlineRegionSizes The computed region sizes to report. + * @return {@code false} if FileSystemUtilizationChore should pause reporting to master, + * {@code true} otherwise. */ boolean reportRegionSizesToMaster(Map<HRegionInfo,Long> onlineRegionSizes) { return this.rs.reportRegionSizesForQuotas(onlineRegionSizes); http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java index 0622dba..1fb8cf4 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java @@ -580,19 +580,19 @@ public class MasterQuotaManager implements RegionStateListener { @VisibleForTesting void initializeRegionSizes() { - assert null == regionSizes; + assert regionSizes == null; this.regionSizes = new ConcurrentHashMap<>(); } public void addRegionSize(HRegionInfo hri, long size, long time) { - if (null == regionSizes) { + if (regionSizes == null) { return; } regionSizes.put(hri, new SizeSnapshotWithTimestamp(size, time)); } public Map<HRegionInfo, Long> snapshotRegionSizes() { - if (null == regionSizes) { + if (regionSizes == null) { return EMPTY_MAP; } @@ -604,7 +604,7 @@ public class MasterQuotaManager implements RegionStateListener { } int pruneEntriesOlderThan(long timeToPruneBefore) { - if (null == regionSizes) { + if (regionSizes == null) { return 0; } int numEntriesRemoved = 0; http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterSpaceQuotaObserver.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterSpaceQuotaObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterSpaceQuotaObserver.java index a3abf32..299ba39 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterSpaceQuotaObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterSpaceQuotaObserver.java @@ -57,7 +57,7 @@ public class MasterSpaceQuotaObserver implements MasterObserver { final MasterServices master = ctx.getEnvironment().getMasterServices(); final Connection conn = master.getConnection(); Quotas quotas = QuotaUtil.getTableQuota(master.getConnection(), tableName); - if (null != quotas && quotas.hasSpace()) { + if (quotas != null && quotas.hasSpace()) { QuotaSettings settings = QuotaSettingsFactory.removeTableSpaceLimit(tableName); try (Admin admin = conn.getAdmin()) { admin.setQuota(settings); @@ -75,7 +75,7 @@ public class MasterSpaceQuotaObserver implements MasterObserver { final MasterServices master = ctx.getEnvironment().getMasterServices(); final Connection conn = master.getConnection(); Quotas quotas = QuotaUtil.getNamespaceQuota(master.getConnection(), namespace); - if (null != quotas && quotas.hasSpace()) { + if (quotas != null && quotas.hasSpace()) { QuotaSettings settings = QuotaSettingsFactory.removeNamespaceSpaceLimit(namespace); try (Admin admin = conn.getAdmin()) { admin.setQuota(settings); http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/NamespaceQuotaSnapshotStore.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/NamespaceQuotaSnapshotStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/NamespaceQuotaSnapshotStore.java index 75550f3..f93d33d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/NamespaceQuotaSnapshotStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/NamespaceQuotaSnapshotStore.java @@ -57,7 +57,7 @@ public class NamespaceQuotaSnapshotStore implements QuotaSnapshotStore<String> { @Override public SpaceQuota getSpaceQuota(String namespace) throws IOException { Quotas quotas = getQuotaForNamespace(namespace); - if (null != quotas && quotas.hasSpace()) { + if (quotas != null && quotas.hasSpace()) { return quotas.getSpace(); } return null; http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java index 254f2a1..4404b27 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaObserverChore.java @@ -136,7 +136,7 @@ public class QuotaObserverChore extends ScheduledChore { } long start = System.nanoTime(); _chore(); - if (null != metrics) { + if (metrics != null) { metrics.incrementQuotaObserverTime((System.nanoTime() - start) / 1_000_000); } } catch (IOException e) { @@ -152,7 +152,7 @@ public class QuotaObserverChore extends ScheduledChore { LOG.trace("Found following tables with quotas: " + tablesWithQuotas); } - if (null != metrics) { + if (metrics != null) { // Set the number of namespaces and tables with quotas defined metrics.setNumSpaceQuotas(tablesWithQuotas.getTableQuotaTables().size() + tablesWithQuotas.getNamespacesWithQuotas().size()); @@ -170,7 +170,7 @@ public class QuotaObserverChore extends ScheduledChore { // Create the stores to track table and namespace snapshots initializeSnapshotStores(reportedRegionSpaceUse); // Report the number of (non-expired) region size reports - if (null != metrics) { + if (metrics != null) { metrics.setNumRegionSizeReports(reportedRegionSpaceUse.size()); } @@ -216,12 +216,12 @@ public class QuotaObserverChore extends ScheduledChore { void initializeSnapshotStores(Map<HRegionInfo,Long> regionSizes) { Map<HRegionInfo,Long> immutableRegionSpaceUse = Collections.unmodifiableMap(regionSizes); - if (null == tableSnapshotStore) { + if (tableSnapshotStore == null) { tableSnapshotStore = new TableQuotaSnapshotStore(conn, this, immutableRegionSpaceUse); } else { tableSnapshotStore.setRegionUsage(immutableRegionSpaceUse); } - if (null == namespaceSnapshotStore) { + if (namespaceSnapshotStore == null) { namespaceSnapshotStore = new NamespaceQuotaSnapshotStore( conn, this, immutableRegionSpaceUse); } else { @@ -239,7 +239,7 @@ public class QuotaObserverChore extends ScheduledChore { long numTablesInViolation = 0L; for (TableName table : tablesWithTableQuotas) { final SpaceQuota spaceQuota = tableSnapshotStore.getSpaceQuota(table); - if (null == spaceQuota) { + if (spaceQuota == null) { if (LOG.isDebugEnabled()) { LOG.debug("Unexpectedly did not find a space quota for " + table + ", maybe it was recently deleted."); @@ -259,7 +259,7 @@ public class QuotaObserverChore extends ScheduledChore { } } // Report the number of tables in violation - if (null != metrics) { + if (metrics != null) { metrics.setNumTableInSpaceQuotaViolation(numTablesInViolation); } } @@ -281,7 +281,7 @@ public class QuotaObserverChore extends ScheduledChore { for (String namespace : namespacesWithQuotas) { // Get the quota definition for the namespace final SpaceQuota spaceQuota = namespaceSnapshotStore.getSpaceQuota(namespace); - if (null == spaceQuota) { + if (spaceQuota == null) { if (LOG.isDebugEnabled()) { LOG.debug("Could not get Namespace space quota for " + namespace + ", maybe it was recently deleted."); @@ -303,7 +303,7 @@ public class QuotaObserverChore extends ScheduledChore { } // Report the number of namespaces in violation - if (null != metrics) { + if (metrics != null) { metrics.setNumNamespacesInSpaceQuotaViolation(numNamespacesInViolation); } } @@ -451,9 +451,8 @@ public class QuotaObserverChore extends ScheduledChore { */ TablesWithQuotas fetchAllTablesWithQuotasDefined() throws IOException { final Scan scan = QuotaTableUtil.makeScan(null); - final QuotaRetriever scanner = new QuotaRetriever(); final TablesWithQuotas tablesWithQuotas = new TablesWithQuotas(conn, conf); - try { + try (final QuotaRetriever scanner = new QuotaRetriever()) { scanner.init(conn, scan); for (QuotaSettings quotaSettings : scanner) { // Only one of namespace and tablename should be 'null' @@ -463,11 +462,10 @@ public class QuotaObserverChore extends ScheduledChore { continue; } - if (null != namespace) { - assert null == tableName; + if (namespace != null) { + assert tableName == null; // Collect all of the tables in the namespace - TableName[] tablesInNS = conn.getAdmin() - .listTableNamesByNamespace(namespace); + TableName[] tablesInNS = conn.getAdmin().listTableNamesByNamespace(namespace); for (TableName tableUnderNs : tablesInNS) { if (LOG.isTraceEnabled()) { LOG.trace("Adding " + tableUnderNs + " under " + namespace @@ -476,7 +474,7 @@ public class QuotaObserverChore extends ScheduledChore { tablesWithQuotas.addNamespaceQuotaTable(tableUnderNs); } } else { - assert null != tableName; + assert tableName != null; if (LOG.isTraceEnabled()) { LOG.trace("Adding " + tableName + " as having table quota."); } @@ -485,10 +483,6 @@ public class QuotaObserverChore extends ScheduledChore { } } return tablesWithQuotas; - } finally { - if (null != scanner) { - scanner.close(); - } } } @@ -504,7 +498,7 @@ public class QuotaObserverChore extends ScheduledChore { /** * Returns an unmodifiable view over the current {@link SpaceQuotaSnapshot} objects - * for each HBase table with a quota. + * for each HBase table with a quota defined. */ public Map<TableName,SpaceQuotaSnapshot> getTableQuotaSnapshots() { return readOnlyTableQuotaSnapshots; @@ -512,7 +506,7 @@ public class QuotaObserverChore extends ScheduledChore { /** * Returns an unmodifiable view over the current {@link SpaceQuotaSnapshot} objects - * for each HBase namespace with a quota. + * for each HBase namespace with a quota defined. */ public Map<String,SpaceQuotaSnapshot> getNamespaceQuotaSnapshots() { return readOnlyNamespaceSnapshots; @@ -522,9 +516,8 @@ public class QuotaObserverChore extends ScheduledChore { * Fetches the {@link SpaceQuotaSnapshot} for the given table. */ SpaceQuotaSnapshot getTableQuotaSnapshot(TableName table) { - // TODO Can one instance of a Chore be executed concurrently? SpaceQuotaSnapshot state = this.tableQuotaSnapshots.get(table); - if (null == state) { + if (state == null) { // No tracked state implies observance. return QuotaSnapshotStore.NO_QUOTA; } @@ -539,12 +532,11 @@ public class QuotaObserverChore extends ScheduledChore { } /** - * Fetches the {@link SpaceQuotaSnapshot} for the given namespace. + * Fetches the {@link SpaceQuotaSnapshot} for the given namespace from this chore. */ SpaceQuotaSnapshot getNamespaceQuotaSnapshot(String namespace) { - // TODO Can one instance of a Chore be executed concurrently? SpaceQuotaSnapshot state = this.namespaceQuotaSnapshots.get(namespace); - if (null == state) { + if (state == null) { // No tracked state implies observance. return QuotaSnapshotStore.NO_QUOTA; } @@ -552,7 +544,7 @@ public class QuotaObserverChore extends ScheduledChore { } /** - * Stores the quota state for the given namespace. + * Stores the given {@code snapshot} for the given {@code namespace} in this chore. */ void setNamespaceQuotaSnapshot(String namespace, SpaceQuotaSnapshot snapshot) { this.namespaceQuotaSnapshots.put(namespace, snapshot); @@ -562,7 +554,8 @@ public class QuotaObserverChore extends ScheduledChore { * Extracts the period for the chore from the configuration. * * @param conf The configuration object. - * @return The configured chore period or the default value. + * @return The configured chore period or the default value in the given timeunit. + * @see #getTimeUnit(Configuration) */ static int getPeriod(Configuration conf) { return conf.getInt(QUOTA_OBSERVER_CHORE_PERIOD_KEY, @@ -573,7 +566,8 @@ public class QuotaObserverChore extends ScheduledChore { * Extracts the initial delay for the chore from the configuration. * * @param conf The configuration object. - * @return The configured chore initial delay or the default value. + * @return The configured chore initial delay or the default value in the given timeunit. + * @see #getTimeUnit(Configuration) */ static long getInitialDelay(Configuration conf) { return conf.getLong(QUOTA_OBSERVER_CHORE_DELAY_KEY, @@ -606,8 +600,8 @@ public class QuotaObserverChore extends ScheduledChore { } /** - * A container which encapsulates the tables which have a table quota and the tables which - * are contained in a namespace which have a namespace quota. + * A container which encapsulates the tables that have either a table quota or are contained in a + * namespace which have a namespace quota. */ static class TablesWithQuotas { private final Set<TableName> tablesWithTableQuotas = new HashSet<>(); @@ -702,7 +696,7 @@ public class QuotaObserverChore extends ScheduledChore { } final int numRegionsInTable = getNumRegions(table); // If the table doesn't exist (no regions), bail out. - if (0 == numRegionsInTable) { + if (numRegionsInTable == 0) { if (LOG.isTraceEnabled()) { LOG.trace("Filtering " + table + " because no regions were reported"); } @@ -734,7 +728,7 @@ public class QuotaObserverChore extends ScheduledChore { */ int getNumRegions(TableName table) throws IOException { List<HRegionInfo> regions = this.conn.getAdmin().getTableRegions(table); - if (null == regions) { + if (regions == null) { return 0; } return regions.size(); http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/RegionServerSpaceQuotaManager.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/RegionServerSpaceQuotaManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/RegionServerSpaceQuotaManager.java index 1c82808..86bdf08 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/RegionServerSpaceQuotaManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/RegionServerSpaceQuotaManager.java @@ -85,7 +85,7 @@ public class RegionServerSpaceQuotaManager { } public synchronized void stop() { - if (null != spaceQuotaRefresher) { + if (spaceQuotaRefresher != null) { spaceQuotaRefresher.cancel(); spaceQuotaRefresher = null; } @@ -133,7 +133,7 @@ public class RegionServerSpaceQuotaManager { final Map<TableName, SpaceQuotaSnapshot> policies = new HashMap<>(); for (Entry<TableName, SpaceViolationPolicyEnforcement> entry : enforcements.entrySet()) { final SpaceQuotaSnapshot snapshot = entry.getValue().getQuotaSnapshot(); - if (null != snapshot) { + if (snapshot != null) { policies.put(entry.getKey(), snapshot); } } @@ -158,9 +158,10 @@ public class RegionServerSpaceQuotaManager { final SpaceViolationPolicyEnforcement enforcement = getFactory().create( getRegionServerServices(), tableName, snapshot); // "Enables" the policy - // TODO Should this synchronize on the actual table name instead of the map? That would allow - // policy enable/disable on different tables to happen concurrently. As written now, only one - // table will be allowed to transition at a time. + // HBASE-XXXX: Should this synchronize on the actual table name instead of the map? That would + // allow policy enable/disable on different tables to happen concurrently. As written now, only + // one table will be allowed to transition at a time. This is probably OK, but not sure if + // it would become a bottleneck at large clusters/number of tables. synchronized (enforcedPolicies) { try { enforcement.enable(); @@ -181,10 +182,9 @@ public class RegionServerSpaceQuotaManager { LOG.trace("Disabling violation policy enforcement on " + tableName); } // "Disables" the policy - // TODO Should this synchronize on the actual table name instead of the map? synchronized (enforcedPolicies) { SpaceViolationPolicyEnforcement enforcement = enforcedPolicies.remove(tableName); - if (null != enforcement) { + if (enforcement != null) { try { enforcement.disable(); } catch (IOException e) { @@ -205,7 +205,7 @@ public class RegionServerSpaceQuotaManager { */ public boolean areCompactionsDisabled(TableName tableName) { SpaceViolationPolicyEnforcement enforcement = this.enforcedPolicies.get(Objects.requireNonNull(tableName)); - if (null != enforcement) { + if (enforcement != null) { return enforcement.areCompactionsDisabled(); } return false; http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceLimitingException.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceLimitingException.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceLimitingException.java index 904903f..e4262c3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceLimitingException.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceLimitingException.java @@ -39,7 +39,7 @@ public class SpaceLimitingException extends QuotaExceededException { // Hack around ResponseConverter expecting to invoke a single-arg String constructor // on this class - if (null != msg) { + if (msg != null) { for (SpaceViolationPolicy definedPolicy : SpaceViolationPolicy.values()) { if (msg.indexOf(definedPolicy.name()) != -1) { policyName = definedPolicy.name(); @@ -74,7 +74,7 @@ public class SpaceLimitingException extends QuotaExceededException { // exists. Best effort... Looks something like: // "org.apache.hadoop.hbase.quotas.SpaceLimitingException: NO_INSERTS A Put is disallowed due // to a space quota." - if (null != originalMessage && originalMessage.startsWith(MESSAGE_PREFIX)) { + if (originalMessage != null && originalMessage.startsWith(MESSAGE_PREFIX)) { // If it starts with the class name, rip off the policy too. try { int index = originalMessage.indexOf(' ', MESSAGE_PREFIX.length()); @@ -90,6 +90,6 @@ public class SpaceLimitingException extends QuotaExceededException { @Override public String getMessage() { - return (null == policyName ? "(unknown policy)" : policyName) + " " + super.getMessage(); + return (policyName == null ? "(unknown policy)" : policyName) + " " + super.getMessage(); } } http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceQuotaRefresherChore.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceQuotaRefresherChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceQuotaRefresherChore.java index 8587e79..5adb9ca 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceQuotaRefresherChore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceQuotaRefresherChore.java @@ -133,7 +133,7 @@ public class SpaceQuotaRefresherChore extends ScheduledChore { * @return true if the snapshot is in violation, false otherwise. */ boolean isInViolation(SpaceQuotaSnapshot snapshot) { - if (null == snapshot) { + if (snapshot == null) { return false; } return snapshot.getQuotaStatus().isInViolation(); http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceViolationPolicyEnforcementFactory.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceViolationPolicyEnforcementFactory.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceViolationPolicyEnforcementFactory.java index 6b754b9..4f1551f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceViolationPolicyEnforcementFactory.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/SpaceViolationPolicyEnforcementFactory.java @@ -20,8 +20,9 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.classification.InterfaceStability; import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshot.SpaceQuotaStatus; -import org.apache.hadoop.hbase.quotas.policies.BulkLoadVerifyingViolationPolicyEnforcement; +import org.apache.hadoop.hbase.quotas.policies.DefaultViolationPolicyEnforcement; import org.apache.hadoop.hbase.quotas.policies.DisableTableViolationPolicyEnforcement; +import org.apache.hadoop.hbase.quotas.policies.MissingSnapshotViolationPolicyEnforcement; import org.apache.hadoop.hbase.quotas.policies.NoInsertsViolationPolicyEnforcement; import org.apache.hadoop.hbase.quotas.policies.NoWritesCompactionsViolationPolicyEnforcement; import org.apache.hadoop.hbase.quotas.policies.NoWritesViolationPolicyEnforcement; @@ -79,16 +80,29 @@ public class SpaceViolationPolicyEnforcementFactory { /** * Creates the "default" {@link SpaceViolationPolicyEnforcement} for a table that isn't in - * violation. This is used to have uniform policy checking for tables in and not quotas. + * violation. This is used to have uniform policy checking for tables in and not quotas. This + * policy will still verify that new bulk loads do not exceed the configured quota limit. + * + * @param rss RegionServerServices instance the policy enforcement should use. + * @param tableName The target HBase table. + * @param snapshot The current quota snapshot for the {@code tableName}, can be null. */ public SpaceViolationPolicyEnforcement createWithoutViolation( RegionServerServices rss, TableName tableName, SpaceQuotaSnapshot snapshot) { + if (snapshot == null) { + // If we have no snapshot, this is equivalent to no quota for this table. + // We should do use the (singleton instance) of this policy to do nothing. + return MissingSnapshotViolationPolicyEnforcement.getInstance(); + } + // We have a snapshot which means that there is a quota set on this table, but it's not in + // violation of that quota. We need to construct a policy for this table. SpaceQuotaStatus status = snapshot.getQuotaStatus(); if (status.isInViolation()) { throw new IllegalArgumentException( tableName + " is in violation. Logic error. Snapshot=" + snapshot); } - BulkLoadVerifyingViolationPolicyEnforcement enforcement = new BulkLoadVerifyingViolationPolicyEnforcement(); + // We have a unique size snapshot to use. Create an instance for this tablename + snapshot. + DefaultViolationPolicyEnforcement enforcement = new DefaultViolationPolicyEnforcement(); enforcement.initialize(rss, tableName, snapshot); return enforcement; } http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/TableQuotaSnapshotStore.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/TableQuotaSnapshotStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/TableQuotaSnapshotStore.java index 82d3684..1abf347 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/TableQuotaSnapshotStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/TableQuotaSnapshotStore.java @@ -58,7 +58,7 @@ public class TableQuotaSnapshotStore implements QuotaSnapshotStore<TableName> { @Override public SpaceQuota getSpaceQuota(TableName subject) throws IOException { Quotas quotas = getQuotaForTable(subject); - if (null != quotas && quotas.hasSpace()) { + if (quotas != null && quotas.hasSpace()) { return quotas.getSpace(); } return null; http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/AbstractViolationPolicyEnforcement.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/AbstractViolationPolicyEnforcement.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/AbstractViolationPolicyEnforcement.java index 2d34d45..981dfd1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/AbstractViolationPolicyEnforcement.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/AbstractViolationPolicyEnforcement.java @@ -16,17 +16,11 @@ */ package org.apache.hadoop.hbase.quotas.policies; -import java.io.IOException; -import java.util.List; import java.util.Objects; -import org.apache.hadoop.fs.FileStatus; -import org.apache.hadoop.fs.FileSystem; -import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.classification.InterfaceStability; -import org.apache.hadoop.hbase.quotas.SpaceLimitingException; import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshot; import org.apache.hadoop.hbase.quotas.SpaceViolationPolicyEnforcement; import org.apache.hadoop.hbase.regionserver.RegionServerServices; @@ -69,7 +63,8 @@ public abstract class AbstractViolationPolicyEnforcement } @Override - public void initialize(RegionServerServices rss, TableName tableName, SpaceQuotaSnapshot snapshot) { + public void initialize( + RegionServerServices rss, TableName tableName, SpaceQuotaSnapshot snapshot) { setRegionServerServices(rss); setTableName(tableName); setQuotaSnapshot(snapshot); @@ -79,40 +74,4 @@ public abstract class AbstractViolationPolicyEnforcement public boolean areCompactionsDisabled() { return false; } - - @Override - public boolean shouldCheckBulkLoads() { - // Reference check. The singleton is used when no quota exists to check against - return SpaceQuotaSnapshot.getNoSuchSnapshot() != quotaSnapshot; - } - - @Override - public void checkBulkLoad(FileSystem fs, List<String> paths) throws SpaceLimitingException { - // Compute the amount of space that could be used to save some arithmetic in the for-loop - final long sizeAvailableForBulkLoads = quotaSnapshot.getLimit() - quotaSnapshot.getUsage(); - long size = 0L; - for (String path : paths) { - size += addSingleFile(fs, path); - if (size > sizeAvailableForBulkLoads) { - break; - } - } - if (size > sizeAvailableForBulkLoads) { - throw new SpaceLimitingException(getPolicyName(), "Bulk load of " + paths - + " is disallowed because the file(s) exceed the limits of a space quota."); - } - } - - private long addSingleFile(FileSystem fs, String path) throws SpaceLimitingException { - final FileStatus status; - try { - status = fs.getFileStatus(new Path(Objects.requireNonNull(path))); - } catch (IOException e) { - throw new SpaceLimitingException(getPolicyName(), "Could not verify length of file to bulk load", e); - } - if (!status.isFile()) { - throw new IllegalArgumentException(path + " is not a file."); - } - return status.getLen(); - } } http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/BulkLoadVerifyingViolationPolicyEnforcement.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/BulkLoadVerifyingViolationPolicyEnforcement.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/BulkLoadVerifyingViolationPolicyEnforcement.java deleted file mode 100644 index e4171ad..0000000 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/BulkLoadVerifyingViolationPolicyEnforcement.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * 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.quotas.policies; - -import org.apache.hadoop.hbase.classification.InterfaceAudience; -import org.apache.hadoop.hbase.client.Mutation; -import org.apache.hadoop.hbase.quotas.SpaceLimitingException; -import org.apache.hadoop.hbase.quotas.SpaceViolationPolicyEnforcement; - -/** - * A {@link SpaceViolationPolicyEnforcement} instance which only checks for bulk loads. Useful for tables - * which have no violation policy. This is the default case for tables, as we want to make sure that - * a single bulk load call would violate the quota. - */ -@InterfaceAudience.Private -public class BulkLoadVerifyingViolationPolicyEnforcement extends AbstractViolationPolicyEnforcement { - - @Override - public void enable() {} - - @Override - public void disable() {} - - @Override - public String getPolicyName() { - return "BulkLoadVerifying"; - } - - @Override - public boolean areCompactionsDisabled() { - return false; - } - - @Override - public void check(Mutation m) throws SpaceLimitingException {} -} http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/DefaultViolationPolicyEnforcement.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/DefaultViolationPolicyEnforcement.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/DefaultViolationPolicyEnforcement.java new file mode 100644 index 0000000..f0c4b53 --- /dev/null +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/DefaultViolationPolicyEnforcement.java @@ -0,0 +1,90 @@ +/* + * 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.quotas.policies; + +import java.io.IOException; +import java.util.List; +import java.util.Objects; + +import org.apache.hadoop.fs.FileStatus; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.classification.InterfaceAudience; +import org.apache.hadoop.hbase.client.Mutation; +import org.apache.hadoop.hbase.quotas.SpaceLimitingException; +import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshot; +import org.apache.hadoop.hbase.quotas.SpaceViolationPolicyEnforcement; + +/** + * The default implementation for {@link SpaceViolationPolicyEnforcement}. This is done because all + * tables, whether or not they're in violation now, should be checking bulk loads to proactively + * catch a swell of files that would push the table into violation. + */ +@InterfaceAudience.Private +public class DefaultViolationPolicyEnforcement extends AbstractViolationPolicyEnforcement { + + @Override + public void enable() throws IOException {} + + @Override + public void disable() throws IOException {} + + @Override + public String getPolicyName() { + return "BulkLoadVerifying"; + } + + @Override + public void check(Mutation m) throws SpaceLimitingException {} + + @Override + public boolean shouldCheckBulkLoads() { + // Reference check. The singleton is used when no quota exists to check against + return SpaceQuotaSnapshot.getNoSuchSnapshot() != quotaSnapshot; + } + + @Override + public void checkBulkLoad(FileSystem fs, List<String> paths) throws SpaceLimitingException { + // Compute the amount of space that could be used to save some arithmetic in the for-loop + final long sizeAvailableForBulkLoads = quotaSnapshot.getLimit() - quotaSnapshot.getUsage(); + long size = 0L; + for (String path : paths) { + size += addSingleFile(fs, path); + if (size > sizeAvailableForBulkLoads) { + break; + } + } + if (size > sizeAvailableForBulkLoads) { + throw new SpaceLimitingException(getPolicyName(), "Bulk load of " + paths + + " is disallowed because the file(s) exceed the limits of a space quota."); + } + } + + private long addSingleFile(FileSystem fs, String path) throws SpaceLimitingException { + final FileStatus status; + try { + status = fs.getFileStatus(new Path(Objects.requireNonNull(path))); + } catch (IOException e) { + throw new SpaceLimitingException( + getPolicyName(), "Could not verify length of file to bulk load", e); + } + if (!status.isFile()) { + throw new IllegalArgumentException(path + " is not a file."); + } + return status.getLen(); + } +} http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/DisableTableViolationPolicyEnforcement.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/DisableTableViolationPolicyEnforcement.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/DisableTableViolationPolicyEnforcement.java index 0d6d886..eb37866 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/DisableTableViolationPolicyEnforcement.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/DisableTableViolationPolicyEnforcement.java @@ -33,7 +33,7 @@ import org.apache.hadoop.hbase.quotas.SpaceViolationPolicyEnforcement; * counterpart to {@link SpaceViolationPolicy#DISABLE}. */ @InterfaceAudience.Private -public class DisableTableViolationPolicyEnforcement extends AbstractViolationPolicyEnforcement { +public class DisableTableViolationPolicyEnforcement extends DefaultViolationPolicyEnforcement { private static final Log LOG = LogFactory.getLog(DisableTableViolationPolicyEnforcement.class); @Override http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/MissingSnapshotViolationPolicyEnforcement.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/MissingSnapshotViolationPolicyEnforcement.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/MissingSnapshotViolationPolicyEnforcement.java new file mode 100644 index 0000000..d0e4b16 --- /dev/null +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/MissingSnapshotViolationPolicyEnforcement.java @@ -0,0 +1,63 @@ +/* + * 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.quotas.policies; + +import java.io.IOException; +import java.util.List; + +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.hbase.client.Mutation; +import org.apache.hadoop.hbase.quotas.SpaceLimitingException; +import org.apache.hadoop.hbase.quotas.SpaceViolationPolicyEnforcement; + +/** + * A {@link SpaceViolationPolicyEnforcement} which can be treated as a singleton. When a quota is + * not defined on a table or we lack quota information, we want to avoid creating a policy, keeping + * this path fast. + */ +public class MissingSnapshotViolationPolicyEnforcement extends AbstractViolationPolicyEnforcement { + private static final MissingSnapshotViolationPolicyEnforcement SINGLETON = + new MissingSnapshotViolationPolicyEnforcement(); + + private MissingSnapshotViolationPolicyEnforcement() {} + + public static SpaceViolationPolicyEnforcement getInstance() { + return SINGLETON; + } + + @Override + public boolean shouldCheckBulkLoads() { + return false; + } + + @Override + public void checkBulkLoad(FileSystem fs, List<String> paths) {} + + @Override + public void enable() throws IOException {} + + @Override + public void disable() throws IOException {} + + @Override + public void check(Mutation m) throws SpaceLimitingException {} + + @Override + public String getPolicyName() { + return "NoQuota"; + } +} http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/NoInsertsViolationPolicyEnforcement.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/NoInsertsViolationPolicyEnforcement.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/NoInsertsViolationPolicyEnforcement.java index a60cb45..ed17cac 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/NoInsertsViolationPolicyEnforcement.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/NoInsertsViolationPolicyEnforcement.java @@ -30,7 +30,7 @@ import org.apache.hadoop.hbase.quotas.SpaceViolationPolicyEnforcement; * enforcement counterpart to {@link SpaceViolationPolicy#NO_INSERTS}. */ @InterfaceAudience.Private -public class NoInsertsViolationPolicyEnforcement extends AbstractViolationPolicyEnforcement { +public class NoInsertsViolationPolicyEnforcement extends DefaultViolationPolicyEnforcement { @Override public void enable() {} http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/NoWritesViolationPolicyEnforcement.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/NoWritesViolationPolicyEnforcement.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/NoWritesViolationPolicyEnforcement.java index a04f418..2ceb051 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/NoWritesViolationPolicyEnforcement.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/policies/NoWritesViolationPolicyEnforcement.java @@ -31,7 +31,7 @@ import org.apache.hadoop.hbase.quotas.SpaceViolationPolicyEnforcement; * into HBase. The enforcement counterpart to {@link SpaceViolationPolicy#NO_WRITES}. */ @InterfaceAudience.Private -public class NoWritesViolationPolicyEnforcement extends AbstractViolationPolicyEnforcement { +public class NoWritesViolationPolicyEnforcement extends DefaultViolationPolicyEnforcement { @Override public void enable() {} http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java index 655d157..7791ea7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java @@ -316,7 +316,7 @@ public class CompactSplitThread implements CompactionRequestor, PropagatingConfi final RegionServerSpaceQuotaManager spaceQuotaManager = this.server.getRegionServerSpaceQuotaManager(); - if (null != spaceQuotaManager && spaceQuotaManager.areCompactionsDisabled( + if (spaceQuotaManager != null && spaceQuotaManager.areCompactionsDisabled( r.getTableDesc().getTableName())) { if (LOG.isDebugEnabled()) { LOG.debug("Ignoring compaction request for " + r + " as an active space quota violation " http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index a50c8c1..3ca061a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java @@ -1324,8 +1324,8 @@ public class HRegionServer extends HasThread implements */ RegionSpaceUse convertRegionSize(HRegionInfo regionInfo, Long sizeInBytes) { return RegionSpaceUse.newBuilder() - .setRegion(HRegionInfo.convert(Objects.requireNonNull(regionInfo))) - .setSize(Objects.requireNonNull(sizeInBytes)) + .setRegionInfo(HRegionInfo.convert(Objects.requireNonNull(regionInfo))) + .setRegionSize(Objects.requireNonNull(sizeInBytes)) .build(); } http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index 42c5a00..6168fda 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java @@ -2248,8 +2248,9 @@ public class RSRpcServices implements HBaseRPCErrorHandler, // Check to see if this bulk load would exceed the space quota for this table if (QuotaUtil.isQuotaEnabled(getConfiguration())) { ActivePolicyEnforcement activeSpaceQuotas = getSpaceQuotaManager().getActiveEnforcements(); - SpaceViolationPolicyEnforcement enforcement = activeSpaceQuotas.getPolicyEnforcement(region); - if (null != enforcement) { + SpaceViolationPolicyEnforcement enforcement = activeSpaceQuotas.getPolicyEnforcement( + region); + if (enforcement != null) { // Bulk loads must still be atomic. We must enact all or none. List<String> filePaths = new ArrayList<>(request.getFamilyPathCount()); for (FamilyPath familyPath : request.getFamilyPathList()) { @@ -3380,7 +3381,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, regionServer.getRegionServerSpaceQuotaManager(); final GetSpaceQuotaSnapshotsResponse.Builder builder = GetSpaceQuotaSnapshotsResponse.newBuilder(); - if (null != manager) { + if (manager != null) { final Map<TableName,SpaceQuotaSnapshot> snapshots = manager.copyQuotaSnapshots(); for (Entry<TableName,SpaceQuotaSnapshot> snapshot : snapshots.entrySet()) { builder.addSnapshots(TableQuotaSnapshot.newBuilder() @@ -3404,7 +3405,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, regionServer.getRegionServerSpaceQuotaManager(); final GetSpaceQuotaEnforcementsResponse.Builder builder = GetSpaceQuotaEnforcementsResponse.newBuilder(); - if (null != manager) { + if (manager != null) { ActivePolicyEnforcement enforcements = manager.getActiveEnforcements(); for (Entry<TableName,SpaceViolationPolicyEnforcement> enforcement : enforcements.getPolicies().entrySet()) { http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/main/resources/hbase-webapps/master/table.jsp ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/resources/hbase-webapps/master/table.jsp b/hbase-server/src/main/resources/hbase-webapps/master/table.jsp index c62d3a6..fd656c6 100644 --- a/hbase-server/src/main/resources/hbase-webapps/master/table.jsp +++ b/hbase-server/src/main/resources/hbase-webapps/master/table.jsp @@ -338,15 +338,15 @@ if ( fqtn != null ) { TableName tn = TableName.valueOf(fqtn); SpaceQuotaSnapshot masterSnapshot = null; Quotas quota = QuotaTableUtil.getTableQuota(master.getConnection(), tn); - if (null == quota || !quota.hasSpace()) { + if (quota == null || !quota.hasSpace()) { quota = QuotaTableUtil.getNamespaceQuota(master.getConnection(), tn.getNamespaceAsString()); - if (null != quota) { + if (quota != null) { masterSnapshot = QuotaTableUtil.getCurrentSnapshot(master.getConnection(), tn.getNamespaceAsString()); } } else { masterSnapshot = QuotaTableUtil.getCurrentSnapshot(master.getConnection(), tn); } - if (null != quota && quota.hasSpace()) { + if (quota != null && quota.hasSpace()) { SpaceQuota spaceQuota = quota.getSpace(); %> <tr> @@ -366,7 +366,7 @@ if ( fqtn != null ) { <td><%= spaceQuota.getViolationPolicy() %></td> </tr> <% - if (null != masterSnapshot) { + if (masterSnapshot != null) { %> <tr> <td>Usage</td> http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java index 888978d..b7c51a2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/SpaceQuotaHelperForTests.java @@ -34,6 +34,7 @@ import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.NamespaceDescriptor; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.Waiter.Predicate; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Connection; @@ -43,6 +44,7 @@ import org.apache.hadoop.hbase.util.Bytes; import org.junit.rules.TestName; import com.google.common.collect.HashMultimap; +import com.google.common.collect.Iterables; import com.google.common.collect.Multimap; @InterfaceAudience.Private @@ -69,6 +71,64 @@ public class SpaceQuotaHelperForTests { // Helpers // + /** + * Returns the number of quotas defined in the HBase quota table. + */ + long listNumDefinedQuotas(Connection conn) throws IOException { + QuotaRetriever scanner = QuotaRetriever.open(conn.getConfiguration()); + try { + return Iterables.size(scanner); + } finally { + if (scanner != null) { + scanner.close(); + } + } + } + + /** + * Removes all quotas defined in the HBase quota table. + */ + void removeAllQuotas(Connection conn) throws IOException { + QuotaRetriever scanner = QuotaRetriever.open(conn.getConfiguration()); + try { + for (QuotaSettings quotaSettings : scanner) { + final String namespace = quotaSettings.getNamespace(); + final TableName tableName = quotaSettings.getTableName(); + if (namespace != null) { + LOG.debug("Deleting quota for namespace: " + namespace); + QuotaUtil.deleteNamespaceQuota(conn, namespace); + } else { + assert tableName != null; + LOG.debug("Deleting quota for table: "+ tableName); + QuotaUtil.deleteTableQuota(conn, tableName); + } + } + } finally { + if (scanner != null) { + scanner.close(); + } + } + } + + /** + * Waits 30seconds for the HBase quota table to exist. + */ + void waitForQuotaTable(Connection conn) throws IOException { + waitForQuotaTable(conn, 30_000); + } + + /** + * Waits {@code timeout} milliseconds for the HBase quota table to exist. + */ + void waitForQuotaTable(Connection conn, long timeout) throws IOException { + testUtil.waitFor(timeout, 1000, new Predicate<IOException>() { + @Override + public boolean evaluate() throws IOException { + return conn.getAdmin().tableExists(QuotaUtil.QUOTA_TABLE_NAME); + } + }); + } + void writeData(TableName tn, long sizeInBytes) throws IOException { final Connection conn = testUtil.getConnection(); final Table table = conn.getTable(tn); @@ -213,14 +273,14 @@ public class SpaceQuotaHelperForTests { for (Entry<TableName, QuotaSettings> entry : quotas.entries()) { SpaceLimitSettings settings = (SpaceLimitSettings) entry.getValue(); TableName tn = entry.getKey(); - if (null != settings.getTableName()) { + if (settings.getTableName() != null) { tablesWithTableQuota.add(tn); } - if (null != settings.getNamespace()) { + if (settings.getNamespace() != null) { tablesWithNamespaceQuota.add(tn); } - if (null == settings.getTableName() && null == settings.getNamespace()) { + if (settings.getTableName() == null && settings.getNamespace() == null) { fail("Unexpected table name with null tableName and namespace: " + tn); } } http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestActivePolicyEnforcement.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestActivePolicyEnforcement.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestActivePolicyEnforcement.java index 80363e8..7c1fd17 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestActivePolicyEnforcement.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestActivePolicyEnforcement.java @@ -25,12 +25,16 @@ import static org.mockito.Mockito.mock; import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Map.Entry; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.quotas.policies.NoWritesViolationPolicyEnforcement; import org.apache.hadoop.hbase.regionserver.RegionServerServices; -import org.apache.hadoop.hbase.quotas.policies.BulkLoadVerifyingViolationPolicyEnforcement; +import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshot.SpaceQuotaStatus; +import org.apache.hadoop.hbase.quotas.policies.DefaultViolationPolicyEnforcement; +import org.apache.hadoop.hbase.quotas.policies.MissingSnapshotViolationPolicyEnforcement; import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.junit.Before; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -40,6 +44,13 @@ import org.junit.experimental.categories.Category; @Category(SmallTests.class) public class TestActivePolicyEnforcement { + private RegionServerServices rss; + + @Before + public void setup() { + rss = mock(RegionServerServices.class); + } + @Test public void testGetter() { final TableName tableName = TableName.valueOf("table"); @@ -57,8 +68,9 @@ public class TestActivePolicyEnforcement { TableName.valueOf("nonexistent")); assertNotNull(enforcement); assertTrue( - "Expected an instance of NoopViolationPolicyEnforcement", - enforcement instanceof BulkLoadVerifyingViolationPolicyEnforcement); + "Expected an instance of MissingSnapshotViolationPolicyEnforcement, but got " + + enforcement.getClass(), + enforcement instanceof MissingSnapshotViolationPolicyEnforcement); } @Test @@ -71,4 +83,48 @@ public class TestActivePolicyEnforcement { TableName.valueOf("nonexistent")); assertFalse("Should not check bulkloads", enforcement.shouldCheckBulkLoads()); } + + @Test + public void testNoQuotaReturnsSingletonPolicyEnforcement() { + final ActivePolicyEnforcement ape = new ActivePolicyEnforcement( + Collections.emptyMap(), Collections.emptyMap(), rss); + final TableName tableName = TableName.valueOf("my_table"); + SpaceViolationPolicyEnforcement policyEnforcement = ape.getPolicyEnforcement(tableName); + // This should be the same exact instance, the singleton + assertTrue(policyEnforcement == MissingSnapshotViolationPolicyEnforcement.getInstance()); + assertEquals(1, ape.getLocallyCachedPolicies().size()); + Entry<TableName,SpaceViolationPolicyEnforcement> entry = + ape.getLocallyCachedPolicies().entrySet().iterator().next(); + assertTrue(policyEnforcement == entry.getValue()); + } + + @Test + public void testNonViolatingQuotaCachesPolicyEnforcment() { + final Map<TableName,SpaceQuotaSnapshot> snapshots = new HashMap<>(); + final TableName tableName = TableName.valueOf("my_table"); + snapshots.put(tableName, new SpaceQuotaSnapshot(SpaceQuotaStatus.notInViolation(), 0, 1024)); + final ActivePolicyEnforcement ape = new ActivePolicyEnforcement( + Collections.emptyMap(), snapshots, rss); + SpaceViolationPolicyEnforcement policyEnforcement = ape.getPolicyEnforcement(tableName); + assertTrue( + "Found the wrong class: " + policyEnforcement.getClass(), + policyEnforcement instanceof DefaultViolationPolicyEnforcement); + SpaceViolationPolicyEnforcement copy = ape.getPolicyEnforcement(tableName); + assertTrue("Expected the instance to be cached", policyEnforcement == copy); + Entry<TableName,SpaceViolationPolicyEnforcement> entry = + ape.getLocallyCachedPolicies().entrySet().iterator().next(); + assertTrue(policyEnforcement == entry.getValue()); + } + + @Test + public void testViolatingQuotaCachesNothing() { + final TableName tableName = TableName.valueOf("my_table"); + SpaceViolationPolicyEnforcement policyEnforcement = mock(SpaceViolationPolicyEnforcement.class); + final Map<TableName,SpaceViolationPolicyEnforcement> activePolicies = new HashMap<>(); + activePolicies.put(tableName, policyEnforcement); + final ActivePolicyEnforcement ape = new ActivePolicyEnforcement( + activePolicies, Collections.emptyMap(), rss); + assertTrue(ape.getPolicyEnforcement(tableName) == policyEnforcement); + assertEquals(0, ape.getLocallyCachedPolicies().size()); + } } http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserver.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserver.java index a1eee4f..ea59d70 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserver.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestMasterSpaceQuotaObserver.java @@ -19,9 +19,8 @@ package org.apache.hadoop.hbase.quotas; import static org.junit.Assert.assertEquals; import java.io.IOException; +import java.util.concurrent.atomic.AtomicLong; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HColumnDescriptor; @@ -45,8 +44,8 @@ import org.junit.rules.TestName; */ @Category(MediumTests.class) public class TestMasterSpaceQuotaObserver { - private static final Log LOG = LogFactory.getLog(TestSpaceQuotas.class); private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + private static SpaceQuotaHelperForTests helper; @Rule public TestName testName = new TestName(); @@ -66,28 +65,17 @@ public class TestMasterSpaceQuotaObserver { @Before public void removeAllQuotas() throws Exception { + if (helper == null) { + helper = new SpaceQuotaHelperForTests(TEST_UTIL, testName, new AtomicLong()); + } final Connection conn = TEST_UTIL.getConnection(); // Wait for the quota table to be created if (!conn.getAdmin().tableExists(QuotaUtil.QUOTA_TABLE_NAME)) { - do { - LOG.debug("Quota table does not yet exist"); - Thread.sleep(1000); - } while (!conn.getAdmin().tableExists(QuotaUtil.QUOTA_TABLE_NAME)); + helper.waitForQuotaTable(conn); } else { // Or, clean up any quotas from previous test runs. - QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration()); - for (QuotaSettings quotaSettings : scanner) { - final String namespace = quotaSettings.getNamespace(); - final TableName tableName = quotaSettings.getTableName(); - if (null != namespace) { - LOG.debug("Deleting quota for namespace: " + namespace); - QuotaUtil.deleteNamespaceQuota(conn, namespace); - } else { - assert null != tableName; - LOG.debug("Deleting quota for table: "+ tableName); - QuotaUtil.deleteTableQuota(conn, tableName); - } - } + helper.removeAllQuotas(conn); + assertEquals(0, helper.listNumDefinedQuotas(conn)); } } http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaObserverChoreRegionReports.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaObserverChoreRegionReports.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaObserverChoreRegionReports.java index d7cdff9..ae315a8 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaObserverChoreRegionReports.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaObserverChoreRegionReports.java @@ -90,7 +90,7 @@ public class TestQuotaObserverChoreRegionReports { final String FAM1 = "f1"; final HMaster master = TEST_UTIL.getMiniHBaseCluster().getMaster(); // Wait for the master to finish initialization. - while (null == master.getMasterQuotaManager()) { + while (master.getMasterQuotaManager() == null) { LOG.debug("MasterQuotaManager is null, waiting..."); Thread.sleep(500); } @@ -170,7 +170,7 @@ public class TestQuotaObserverChoreRegionReports { @Override public boolean evaluate() throws Exception { SpaceQuotaSnapshot snapshot = getSnapshotForTable(conn, tn); - if (null == snapshot) { + if (snapshot == null) { return false; } return snapshot.getQuotaStatus().isInViolation(); @@ -188,7 +188,7 @@ public class TestQuotaObserverChoreRegionReports { @Override public boolean evaluate() throws Exception { SpaceQuotaSnapshot snapshot = getSnapshotForTable(conn, tn); - if (null == snapshot) { + if (snapshot == null) { return false; } return !snapshot.getQuotaStatus().isInViolation(); http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaObserverChoreWithMiniCluster.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaObserverChoreWithMiniCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaObserverChoreWithMiniCluster.java index 63198a8..dde9e71 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaObserverChoreWithMiniCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaObserverChoreWithMiniCluster.java @@ -95,35 +95,24 @@ public class TestQuotaObserverChoreWithMiniCluster { @Before public void removeAllQuotas() throws Exception { final Connection conn = TEST_UTIL.getConnection(); + if (helper == null) { + helper = new SpaceQuotaHelperForTests(TEST_UTIL, testName, COUNTER); + } // Wait for the quota table to be created if (!conn.getAdmin().tableExists(QuotaUtil.QUOTA_TABLE_NAME)) { - do { - LOG.debug("Quota table does not yet exist"); - Thread.sleep(DEFAULT_WAIT_MILLIS); - } while (!conn.getAdmin().tableExists(QuotaUtil.QUOTA_TABLE_NAME)); + helper.waitForQuotaTable(conn); } else { // Or, clean up any quotas from previous test runs. - QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration()); - for (QuotaSettings quotaSettings : scanner) { - final String namespace = quotaSettings.getNamespace(); - final TableName tableName = quotaSettings.getTableName(); - if (null != namespace) { - LOG.debug("Deleting quota for namespace: " + namespace); - QuotaUtil.deleteNamespaceQuota(conn, namespace); - } else { - assert null != tableName; - LOG.debug("Deleting quota for table: "+ tableName); - QuotaUtil.deleteTableQuota(conn, tableName); - } - } + helper.removeAllQuotas(conn); + assertEquals(0, helper.listNumDefinedQuotas(conn)); } master = TEST_UTIL.getMiniHBaseCluster().getMaster(); snapshotNotifier = (SpaceQuotaSnapshotNotifierForTest) master.getSpaceQuotaSnapshotNotifier(); + assertNotNull(snapshotNotifier); snapshotNotifier.clearSnapshots(); chore = master.getQuotaObserverChore(); - helper = new SpaceQuotaHelperForTests(TEST_UTIL, testName, COUNTER); } @Test @@ -382,7 +371,7 @@ public class TestQuotaObserverChoreWithMiniCluster { @Override int getNumReportedRegions(TableName table, QuotaSnapshotStore<TableName> tableStore) { Integer i = mockReportedRegions.get(table); - if (null == i) { + if (i == null) { return 0; } return i; @@ -424,10 +413,10 @@ public class TestQuotaObserverChoreWithMiniCluster { qs instanceof SpaceLimitSettings); SpaceQuota spaceQuota = null; - if (null != qs.getTableName()) { + if (qs.getTableName() != null) { spaceQuota = chore.getTableSnapshotStore().getSpaceQuota(table); assertNotNull("Could not find table space quota for " + table, spaceQuota); - } else if (null != qs.getNamespace()) { + } else if (qs.getNamespace() != null) { spaceQuota = chore.getNamespaceSnapshotStore().getSpaceQuota(table.getNamespaceAsString()); assertNotNull("Could not find namespace space quota for " + table.getNamespaceAsString(), spaceQuota); } else { http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaStatusRPCs.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaStatusRPCs.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaStatusRPCs.java index 38dbf66..2cd67c9 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaStatusRPCs.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestQuotaStatusRPCs.java @@ -37,6 +37,7 @@ import org.apache.hadoop.hbase.Waiter.Predicate; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshot.SpaceQuotaStatus; +import org.apache.hadoop.hbase.quotas.policies.MissingSnapshotViolationPolicyEnforcement; import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.junit.AfterClass; @@ -132,7 +133,7 @@ public class TestQuotaStatusRPCs { @Override public boolean evaluate() throws Exception { SpaceQuotaSnapshot snapshot = manager.copyQuotaSnapshots().get(tn); - if (null == snapshot) { + if (snapshot == null) { return false; } return snapshot.getUsage() >= tableSize; @@ -177,6 +178,10 @@ public class TestQuotaStatusRPCs { public boolean evaluate() throws Exception { ActivePolicyEnforcement enforcements = manager.getActiveEnforcements(); SpaceViolationPolicyEnforcement enforcement = enforcements.getPolicyEnforcement(tn); + // Signifies that we're waiting on the quota snapshot to be fetched + if (enforcement instanceof MissingSnapshotViolationPolicyEnforcement) { + return false; + } return enforcement.getQuotaSnapshot().getQuotaStatus().isInViolation(); } }); @@ -215,7 +220,7 @@ public class TestQuotaStatusRPCs { public boolean evaluate() throws Exception { SpaceQuotaSnapshot snapshot = QuotaTableUtil.getCurrentSnapshot(conn, tn); LOG.info("Table snapshot after initial ingest: " + snapshot); - if (null == snapshot) { + if (snapshot == null) { return false; } return snapshot.getLimit() == sizeLimit && snapshot.getUsage() > 0L; @@ -229,7 +234,7 @@ public class TestQuotaStatusRPCs { SpaceQuotaSnapshot snapshot = QuotaTableUtil.getCurrentSnapshot( conn, tn.getNamespaceAsString()); LOG.debug("Namespace snapshot after initial ingest: " + snapshot); - if (null == snapshot) { + if (snapshot == null) { return false; } nsUsage.set(snapshot.getUsage()); @@ -249,7 +254,7 @@ public class TestQuotaStatusRPCs { public boolean evaluate() throws Exception { SpaceQuotaSnapshot snapshot = QuotaTableUtil.getCurrentSnapshot(conn, tn); LOG.info("Table snapshot after second ingest: " + snapshot); - if (null == snapshot) { + if (snapshot == null) { return false; } return snapshot.getQuotaStatus().isInViolation(); @@ -262,7 +267,7 @@ public class TestQuotaStatusRPCs { SpaceQuotaSnapshot snapshot = QuotaTableUtil.getCurrentSnapshot( conn, tn.getNamespaceAsString()); LOG.debug("Namespace snapshot after second ingest: " + snapshot); - if (null == snapshot) { + if (snapshot == null) { return false; } return snapshot.getUsage() > nsUsage.get() && !snapshot.getQuotaStatus().isInViolation(); http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestRegionServerSpaceQuotaManager.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestRegionServerSpaceQuotaManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestRegionServerSpaceQuotaManager.java index 38656e8..5f11950 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestRegionServerSpaceQuotaManager.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestRegionServerSpaceQuotaManager.java @@ -33,7 +33,7 @@ import org.apache.hadoop.hbase.quotas.policies.DisableTableViolationPolicyEnforc import org.apache.hadoop.hbase.quotas.policies.NoInsertsViolationPolicyEnforcement; import org.apache.hadoop.hbase.quotas.policies.NoWritesCompactionsViolationPolicyEnforcement; import org.apache.hadoop.hbase.quotas.policies.NoWritesViolationPolicyEnforcement; -import org.apache.hadoop.hbase.quotas.policies.BulkLoadVerifyingViolationPolicyEnforcement; +import org.apache.hadoop.hbase.quotas.policies.DefaultViolationPolicyEnforcement; import org.apache.hadoop.hbase.regionserver.RegionServerServices; import org.apache.hadoop.hbase.testclassification.SmallTests; import org.junit.Before; @@ -95,7 +95,7 @@ public class TestRegionServerSpaceQuotaManager { expectedPolicies.put(disablePolicy.getTableName(), disableSnapshot); enforcements.put( - TableName.valueOf("no_policy"), new BulkLoadVerifyingViolationPolicyEnforcement()); + TableName.valueOf("no_policy"), new DefaultViolationPolicyEnforcement()); Map<TableName, SpaceQuotaSnapshot> actualPolicies = quotaManager.getActivePoliciesAsMap(); assertEquals(expectedPolicies, actualPolicies); http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotas.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotas.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotas.java index ffe0ce2..e21647f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotas.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestSpaceQuotas.java @@ -56,7 +56,7 @@ import org.apache.hadoop.hbase.client.SecureBulkLoadClient; import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.ipc.RpcControllerFactory; import org.apache.hadoop.hbase.master.HMaster; -import org.apache.hadoop.hbase.quotas.policies.BulkLoadVerifyingViolationPolicyEnforcement; +import org.apache.hadoop.hbase.quotas.policies.DefaultViolationPolicyEnforcement; import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.regionserver.TestHRegionServerBulkLoad; import org.apache.hadoop.hbase.security.AccessDeniedException; @@ -108,29 +108,17 @@ public class TestSpaceQuotas { @Before public void removeAllQuotas() throws Exception { final Connection conn = TEST_UTIL.getConnection(); + if (helper == null) { + helper = new SpaceQuotaHelperForTests(TEST_UTIL, testName, COUNTER); + } // Wait for the quota table to be created if (!conn.getAdmin().tableExists(QuotaUtil.QUOTA_TABLE_NAME)) { - do { - LOG.debug("Quota table does not yet exist"); - Thread.sleep(1000); - } while (!conn.getAdmin().tableExists(QuotaUtil.QUOTA_TABLE_NAME)); + helper.waitForQuotaTable(conn); } else { // Or, clean up any quotas from previous test runs. - QuotaRetriever scanner = QuotaRetriever.open(TEST_UTIL.getConfiguration()); - for (QuotaSettings quotaSettings : scanner) { - final String namespace = quotaSettings.getNamespace(); - final TableName tableName = quotaSettings.getTableName(); - if (null != namespace) { - LOG.debug("Deleting quota for namespace: " + namespace); - QuotaUtil.deleteNamespaceQuota(conn, namespace); - } else { - assert null != tableName; - LOG.debug("Deleting quota for table: "+ tableName); - QuotaUtil.deleteTableQuota(conn, tableName); - } - } + helper.removeAllQuotas(conn); + assertEquals(0, helper.listNumDefinedQuotas(conn)); } - helper = new SpaceQuotaHelperForTests(TEST_UTIL, testName, COUNTER); } @Test @@ -285,7 +273,7 @@ public class TestSpaceQuotas { Map<HRegionInfo,Long> regionSizes = getReportedSizesForTable(tn); while (true) { SpaceQuotaSnapshot snapshot = snapshots.get(tn); - if (null != snapshot && snapshot.getLimit() > 0) { + if (snapshot != null && snapshot.getLimit() > 0) { break; } LOG.debug( @@ -305,7 +293,7 @@ public class TestSpaceQuotas { SpaceViolationPolicyEnforcement enforcement = activePolicies.getPolicyEnforcement(tn); assertTrue( "Expected to find Noop policy, but got " + enforcement.getClass().getSimpleName(), - enforcement instanceof BulkLoadVerifyingViolationPolicyEnforcement); + enforcement instanceof DefaultViolationPolicyEnforcement); // Should generate two files, each of which is over 25KB each ClientServiceCallable<Void> callable = generateFileToLoad(tn, 2, 500); http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestTableSpaceQuotaViolationNotifier.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestTableSpaceQuotaViolationNotifier.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestTableSpaceQuotaViolationNotifier.java index d190c8c..6626ab5 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestTableSpaceQuotaViolationNotifier.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestTableSpaceQuotaViolationNotifier.java @@ -68,10 +68,10 @@ public class TestTableSpaceQuotaViolationNotifier { final Put expectedPut = new Put(Bytes.toBytes("t." + tn.getNameAsString())); final QuotaProtos.SpaceQuotaSnapshot protoQuota = QuotaProtos.SpaceQuotaSnapshot.newBuilder() - .setStatus(QuotaProtos.SpaceQuotaStatus.newBuilder().setInViolation(true).setPolicy( - org.apache.hadoop.hbase.shaded.protobuf.generated.QuotaProtos.SpaceViolationPolicy.NO_INSERTS)) - .setLimit(512L) - .setUsage(1024L) + .setQuotaStatus(QuotaProtos.SpaceQuotaStatus.newBuilder().setInViolation(true) + .setViolationPolicy(QuotaProtos.SpaceViolationPolicy.NO_INSERTS)) + .setQuotaLimit(512L) + .setQuotaUsage(1024L) .build(); expectedPut.addColumn(Bytes.toBytes("u"), Bytes.toBytes("p"), protoQuota.toByteArray()); http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/policies/TestBulkLoadCheckingViolationPolicyEnforcement.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/policies/TestBulkLoadCheckingViolationPolicyEnforcement.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/policies/TestBulkLoadCheckingViolationPolicyEnforcement.java index abe1b9d..bd0bc8c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/policies/TestBulkLoadCheckingViolationPolicyEnforcement.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/policies/TestBulkLoadCheckingViolationPolicyEnforcement.java @@ -49,7 +49,7 @@ public class TestBulkLoadCheckingViolationPolicyEnforcement { fs = mock(FileSystem.class); rss = mock(RegionServerServices.class); tableName = TableName.valueOf("foo"); - policy = new BulkLoadVerifyingViolationPolicyEnforcement(); + policy = new DefaultViolationPolicyEnforcement(); } @Test http://git-wip-us.apache.org/repos/asf/hbase/blob/d671a1db/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerRegionSpaceUseReport.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerRegionSpaceUseReport.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerRegionSpaceUseReport.java index 3244681..7c16d32 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerRegionSpaceUseReport.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerRegionSpaceUseReport.java @@ -63,11 +63,11 @@ public class TestRegionServerRegionSpaceUseReport { RegionSpaceUseReportRequest requests = rs.buildRegionSpaceUseReportRequest(sizes); assertEquals(sizes.size(), requests.getSpaceUseCount()); for (RegionSpaceUse spaceUse : requests.getSpaceUseList()) { - RegionInfo ri = spaceUse.getRegion(); + RegionInfo ri = spaceUse.getRegionInfo(); HRegionInfo hri = HRegionInfo.convert(ri); Long expectedSize = sizes.remove(hri); assertNotNull("Could not find size for HRI: " + hri, expectedSize); - assertEquals(expectedSize.longValue(), spaceUse.getSize()); + assertEquals(expectedSize.longValue(), spaceUse.getRegionSize()); } assertTrue("Should not have any space use entries left: " + sizes, sizes.isEmpty()); }
