You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by sn...@apache.org on 2019/06/11 06:13:43 UTC
[incubator-pinot] branch master updated: Clean up table name access
for StorageQuotaChecker (#4302)
This is an automated email from the ASF dual-hosted git repository.
snlee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 1257a35 Clean up table name access for StorageQuotaChecker (#4302)
1257a35 is described below
commit 1257a3517fe46f9540578063d1a1cfd133140ca7
Author: Seunghyun Lee <sn...@linkedin.com>
AuthorDate: Mon Jun 10 23:13:38 2019 -0700
Clean up table name access for StorageQuotaChecker (#4302)
tableConfig.getTableName() already includes the table
type along with the table name. Merging tableName and
tableNameWithType together to a single variable.
---
.../controller/api/upload/SegmentValidator.java | 3 +--
.../controller/validation/StorageQuotaChecker.java | 26 ++++++++++------------
.../validation/StorageQuotaCheckerTest.java | 16 ++++++-------
3 files changed, 21 insertions(+), 24 deletions(-)
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/SegmentValidator.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/SegmentValidator.java
index 6af7af7..39a9657 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/SegmentValidator.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/SegmentValidator.java
@@ -136,8 +136,7 @@ public class SegmentValidator {
StorageQuotaChecker quotaChecker =
new StorageQuotaChecker(offlineTableConfig, tableSizeReader, _controllerMetrics, _pinotHelixResourceManager,
_controllerLeadershipManager);
- String offlineTableName = offlineTableConfig.getTableName();
- return quotaChecker.isSegmentStorageWithinQuota(segmentFile, offlineTableName, metadata.getName(),
+ return quotaChecker.isSegmentStorageWithinQuota(segmentFile, metadata.getName(),
_controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
}
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/validation/StorageQuotaChecker.java b/pinot-controller/src/main/java/org/apache/pinot/controller/validation/StorageQuotaChecker.java
index 156ad9f..db23301 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/validation/StorageQuotaChecker.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/validation/StorageQuotaChecker.java
@@ -82,16 +82,14 @@ public class StorageQuotaChecker {
* check if the segment represented by segmentFile is within the storage quota
* @param segmentFile untarred segment. This should not be null.
* segmentFile must exist on disk and must be a directory
- * @param tableNameWithType table name with type (OFFLINE/REALTIME) information
* @param segmentName name of the segment being added
* @param timeoutMsec timeout in milliseconds for reading table sizes from server
*
*/
- public QuotaCheckerResponse isSegmentStorageWithinQuota(@Nonnull File segmentFile, @Nonnull String tableNameWithType,
- @Nonnull String segmentName, @Nonnegative int timeoutMsec)
+ public QuotaCheckerResponse isSegmentStorageWithinQuota(@Nonnull File segmentFile, @Nonnull String segmentName,
+ @Nonnegative int timeoutMsec)
throws InvalidConfigException {
Preconditions.checkNotNull(segmentFile);
- Preconditions.checkNotNull(tableNameWithType);
Preconditions.checkNotNull(segmentName);
Preconditions.checkArgument(timeoutMsec > 0, "Timeout value must be > 0, input: %s", timeoutMsec);
Preconditions.checkArgument(segmentFile.exists(), "Segment file: %s does not exist", segmentFile);
@@ -103,7 +101,7 @@ public class StorageQuotaChecker {
// 4. is the updated size within quota
QuotaConfig quotaConfig = _tableConfig.getQuotaConfig();
int numReplicas = _tableConfig.getValidationConfig().getReplicationNumber();
- final String tableName = _tableConfig.getTableName();
+ final String tableNameWithType = _tableConfig.getTableName();
if (quotaConfig == null || Strings.isNullOrEmpty(quotaConfig.getStorage())) {
// no quota configuration...so ignore for backwards compatibility
@@ -116,7 +114,7 @@ public class StorageQuotaChecker {
LOGGER.warn("Storage quota is not configured for table: {}", tableNameWithType);
return success("Storage quota is not configured for table: " + tableNameWithType);
}
- _controllerMetrics.setValueOfTableGauge(tableName, ControllerGauge.TABLE_QUOTA, allowedStorageBytes);
+ _controllerMetrics.setValueOfTableGauge(tableNameWithType, ControllerGauge.TABLE_QUOTA, allowedStorageBytes);
long incomingSegmentSizeBytes = FileUtils.sizeOfDirectory(segmentFile);
@@ -152,19 +150,19 @@ public class StorageQuotaChecker {
// Since tableNameWithType comes with the table type(OFFLINE), thus we guarantee that
// tableSubtypeSize.estimatedSizeInBytes is the offline table size.
- _controllerMetrics.setValueOfTableGauge(tableName, ControllerGauge.OFFLINE_TABLE_ESTIMATED_SIZE,
+ _controllerMetrics.setValueOfTableGauge(tableNameWithType, ControllerGauge.OFFLINE_TABLE_ESTIMATED_SIZE,
tableSubtypeSize.estimatedSizeInBytes);
LOGGER.info("Table {}'s estimatedSizeInBytes is {}. ReportedSizeInBytes (actual reports from servers) is {}",
- tableName, tableSubtypeSize.estimatedSizeInBytes, tableSubtypeSize.reportedSizeInBytes);
+ tableNameWithType, tableSubtypeSize.estimatedSizeInBytes, tableSubtypeSize.reportedSizeInBytes);
// Only emit the real percentage of storage quota usage by lead controller, otherwise emit 0L.
if (isLeader() && allowedStorageBytes != 0L) {
long existingStorageQuotaUtilization = tableSubtypeSize.estimatedSizeInBytes * 100 / allowedStorageBytes;
- _controllerMetrics.setValueOfTableGauge(tableName, ControllerGauge.TABLE_STORAGE_QUOTA_UTILIZATION,
+ _controllerMetrics.setValueOfTableGauge(tableNameWithType, ControllerGauge.TABLE_STORAGE_QUOTA_UTILIZATION,
existingStorageQuotaUtilization);
} else {
- _controllerMetrics.setValueOfTableGauge(tableName, ControllerGauge.TABLE_STORAGE_QUOTA_UTILIZATION, 0L);
+ _controllerMetrics.setValueOfTableGauge(tableNameWithType, ControllerGauge.TABLE_STORAGE_QUOTA_UTILIZATION, 0L);
}
// Note: incomingSegmentSizeBytes is uncompressed data size for just 1 replica,
@@ -178,7 +176,7 @@ public class StorageQuotaChecker {
// append use case
message = String.format(
"Appending Segment %s of Table %s is within quota. Total allowed storage size: %s ( = configured quota: %s * number replicas: %d). New estimated table size of all replicas: %s. Current table size of all replicas: %s. Incoming uncompressed segment size of all replicas: %s ( = single incoming uncompressed segment size: %s * number replicas: %d). Formula: New estimated size = current table size + incoming segment size",
- segmentName, tableName, DataSize.fromBytes(allowedStorageBytes),
+ segmentName, tableNameWithType, DataSize.fromBytes(allowedStorageBytes),
DataSize.fromBytes(quotaConfig.storageSizeBytes()), numReplicas,
DataSize.fromBytes(estimatedFinalSizeBytes), DataSize.fromBytes(tableSubtypeSize.estimatedSizeInBytes),
DataSize.fromBytes(totalIncomingSegmentSizeBytes), DataSize.fromBytes(incomingSegmentSizeBytes),
@@ -187,7 +185,7 @@ public class StorageQuotaChecker {
// refresh use case
message = String.format(
"Refreshing Segment %s of Table %s is within quota. Total allowed storage size: %s ( = configured quota: %s * number replicas: %d). New estimated table size of all replicas: %s. Current table size of all replicas: %s. Incoming uncompressed segment size of all replicas: %s ( = single incoming uncompressed segment size: %s * number replicas: %d). Existing same segment size of all replicas: %s. Formula: New estimated size = current table size - existing same segment size + incom [...]
- segmentName, tableName, DataSize.fromBytes(allowedStorageBytes),
+ segmentName, tableNameWithType, DataSize.fromBytes(allowedStorageBytes),
DataSize.fromBytes(quotaConfig.storageSizeBytes()), numReplicas,
DataSize.fromBytes(estimatedFinalSizeBytes), DataSize.fromBytes(tableSubtypeSize.estimatedSizeInBytes),
DataSize.fromBytes(totalIncomingSegmentSizeBytes), DataSize.fromBytes(incomingSegmentSizeBytes),
@@ -200,12 +198,12 @@ public class StorageQuotaChecker {
if (tableSubtypeSize.estimatedSizeInBytes > allowedStorageBytes) {
message = String.format(
"Table %s already over quota. Existing estimated uncompressed table size of all replicas: %s > total allowed storage size: %s ( = configured quota: %s * num replicas: %d). Check if indexes were enabled recently and adjust table quota accordingly.",
- tableName, DataSize.fromBytes(tableSubtypeSize.estimatedSizeInBytes),
+ tableNameWithType, DataSize.fromBytes(tableSubtypeSize.estimatedSizeInBytes),
DataSize.fromBytes(allowedStorageBytes), DataSize.fromBytes(quotaConfig.storageSizeBytes()), numReplicas);
} else {
message = String.format(
"Storage quota exceeded for Table %s. New estimated size: %s > total allowed storage size: %s, where new estimated size = existing estimated uncompressed size of all replicas: %s - existing segment sizes of all replicas: %s + (incoming uncompressed segment size: %s * number replicas: %d), total allowed storage size = configured quota: %s * number replicas: %d",
- tableName, DataSize.fromBytes(estimatedFinalSizeBytes), DataSize.fromBytes(allowedStorageBytes),
+ tableNameWithType, DataSize.fromBytes(estimatedFinalSizeBytes), DataSize.fromBytes(allowedStorageBytes),
DataSize.fromBytes(tableSubtypeSize.estimatedSizeInBytes), DataSize.fromBytes(existingSegmentSizeBytes),
DataSize.fromBytes(incomingSegmentSizeBytes), numReplicas,
DataSize.fromBytes(quotaConfig.storageSizeBytes()), numReplicas);
diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/validation/StorageQuotaCheckerTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/validation/StorageQuotaCheckerTest.java
index ed38a50..e84a947 100644
--- a/pinot-controller/src/test/java/org/apache/pinot/controller/validation/StorageQuotaCheckerTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/validation/StorageQuotaCheckerTest.java
@@ -81,7 +81,7 @@ public class StorageQuotaCheckerTest {
_controllerLeadershipManager);
when(_tableConfig.getQuotaConfig()).thenReturn(null);
StorageQuotaChecker.QuotaCheckerResponse res =
- checker.isSegmentStorageWithinQuota(TEST_DIR, "myTable", "segment", 1000);
+ checker.isSegmentStorageWithinQuota(TEST_DIR, "segment", 1000);
Assert.assertTrue(res.isSegmentWithinQuota);
}
@@ -94,7 +94,7 @@ public class StorageQuotaCheckerTest {
when(_tableConfig.getQuotaConfig()).thenReturn(_quotaConfig);
when(_quotaConfig.storageSizeBytes()).thenReturn(-1L);
StorageQuotaChecker.QuotaCheckerResponse res =
- checker.isSegmentStorageWithinQuota(TEST_DIR, "myTable", "segment", 1000);
+ checker.isSegmentStorageWithinQuota(TEST_DIR, "segment", 1000);
Assert.assertTrue(res.isSegmentWithinQuota);
}
@@ -136,7 +136,7 @@ public class StorageQuotaCheckerTest {
new MockStorageQuotaChecker(_tableConfig, _tableSizeReader, _controllerMetrics, _pinotHelixResourceManager,
_controllerLeadershipManager);
StorageQuotaChecker.QuotaCheckerResponse response =
- checker.isSegmentStorageWithinQuota(TEST_DIR, tableName, "segment1", 1000);
+ checker.isSegmentStorageWithinQuota(TEST_DIR, "segment1", 1000);
Assert.assertTrue(response.isSegmentWithinQuota);
Assert.assertEquals(
_controllerMetrics.getValueOfTableGauge(tableName, ControllerGauge.TABLE_STORAGE_QUOTA_UTILIZATION), 80L);
@@ -144,7 +144,7 @@ public class StorageQuotaCheckerTest {
// Quota exceeded.
when(_quotaConfig.storageSizeBytes()).thenReturn(2800L);
when(_quotaConfig.getStorage()).thenReturn("2.8K");
- response = checker.isSegmentStorageWithinQuota(TEST_DIR, tableName, "segment1", 1000);
+ response = checker.isSegmentStorageWithinQuota(TEST_DIR, "segment1", 1000);
Assert.assertFalse(response.isSegmentWithinQuota);
Assert.assertEquals(
_controllerMetrics.getValueOfTableGauge(tableName, ControllerGauge.TABLE_STORAGE_QUOTA_UTILIZATION), 85L);
@@ -153,7 +153,7 @@ public class StorageQuotaCheckerTest {
setupTableSegmentSize(6000L, 900L, 0);
when(_quotaConfig.storageSizeBytes()).thenReturn(2800L);
when(_quotaConfig.getStorage()).thenReturn("2.8K");
- response = checker.isSegmentStorageWithinQuota(TEST_DIR, tableName, "segment1", 1000);
+ response = checker.isSegmentStorageWithinQuota(TEST_DIR, "segment1", 1000);
Assert.assertFalse(response.isSegmentWithinQuota);
Assert.assertEquals(
_controllerMetrics.getValueOfTableGauge(tableName, ControllerGauge.TABLE_STORAGE_QUOTA_UTILIZATION), 107L);
@@ -162,21 +162,21 @@ public class StorageQuotaCheckerTest {
setupTableSegmentSize(-1, -1, 0);
when(_quotaConfig.storageSizeBytes()).thenReturn(2800L);
when(_quotaConfig.getStorage()).thenReturn("2.8K");
- response = checker.isSegmentStorageWithinQuota(TEST_DIR, tableName, "segment1", 1000);
+ response = checker.isSegmentStorageWithinQuota(TEST_DIR, "segment1", 1000);
Assert.assertTrue(response.isSegmentWithinQuota);
// partial response from servers, but table already over quota
setupTableSegmentSize(6000L, 900L, -2);
when(_quotaConfig.storageSizeBytes()).thenReturn(2800L);
when(_quotaConfig.getStorage()).thenReturn("2.8K");
- response = checker.isSegmentStorageWithinQuota(TEST_DIR, tableName, "segment1", 1000);
+ response = checker.isSegmentStorageWithinQuota(TEST_DIR, "segment1", 1000);
Assert.assertFalse(response.isSegmentWithinQuota);
// partial response from servers, but current estimate within quota
setupTableSegmentSize(2000L, 900L, -2);
when(_quotaConfig.storageSizeBytes()).thenReturn(2800L);
when(_quotaConfig.getStorage()).thenReturn("2.8K");
- response = checker.isSegmentStorageWithinQuota(TEST_DIR, tableName, "segment1", 1000);
+ response = checker.isSegmentStorageWithinQuota(TEST_DIR, "segment1", 1000);
Assert.assertTrue(response.isSegmentWithinQuota);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org