You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/01/20 00:30:57 UTC
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6465: Dimension table storage quota config and validation
Jackie-Jiang commented on a change in pull request #6465:
URL: https://github.com/apache/incubator-pinot/pull/6465#discussion_r560590258
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java
##########
@@ -145,6 +145,7 @@ private static long getRandomInitialDelayInSeconds() {
// If it's set to false, existing HLC realtime tables will stop consumption, and creation of new HLC tables will be disallowed.
// Please make sure there is no HLC table running in the cluster before disallowing it.
public static final String ALLOW_HLC_TABLES = "controller.allow.hlc.tables";
+ private static final String DIM_TABLE_MAX_SIZE = "controller.dimTable.maxSize";
Review comment:
Make the config key public
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -463,6 +467,36 @@ private void ensureMinReplicas(TableConfig tableConfig) {
}
}
+ private void ensureStorageQuotaConstraints(TableConfig tableConfig) {
+ // Dim tables must adhere to cluster level storage size limits
+ if (tableConfig.isDimTable()) {
+ QuotaConfig quotaConfig = tableConfig.getQuotaConfig();
+ String maxAllowedSize = this._controllerConf.getDimTableMaxSize();
Review comment:
(Convention) Remove `this.`
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -463,6 +467,36 @@ private void ensureMinReplicas(TableConfig tableConfig) {
}
}
+ private void ensureStorageQuotaConstraints(TableConfig tableConfig) {
+ // Dim tables must adhere to cluster level storage size limits
+ if (tableConfig.isDimTable()) {
+ QuotaConfig quotaConfig = tableConfig.getQuotaConfig();
+ String maxAllowedSize = this._controllerConf.getDimTableMaxSize();
+ long maxAllowedSizeInBytes = DataSizeUtils.toBytes(maxAllowedSize);
+
+ if (quotaConfig == null) {
+ // set a default storage quota
+ LOGGER.info("Assigning default storage quota for dimension table: {}", tableConfig.getTableName());
Review comment:
Put the default value in the info message
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -463,6 +467,36 @@ private void ensureMinReplicas(TableConfig tableConfig) {
}
}
+ private void ensureStorageQuotaConstraints(TableConfig tableConfig) {
+ // Dim tables must adhere to cluster level storage size limits
+ if (tableConfig.isDimTable()) {
+ QuotaConfig quotaConfig = tableConfig.getQuotaConfig();
+ String maxAllowedSize = this._controllerConf.getDimTableMaxSize();
+ long maxAllowedSizeInBytes = DataSizeUtils.toBytes(maxAllowedSize);
+
+ if (quotaConfig == null) {
+ // set a default storage quota
+ LOGGER.info("Assigning default storage quota for dimension table: {}", tableConfig.getTableName());
+ tableConfig.setQuotaConfig(
+ new QuotaConfig(maxAllowedSize, null));
+ } else {
+ if (quotaConfig.getStorage() == null) {
+ // set a default storage quota and keep the RPS value
+ LOGGER.info("Assigning default storage quota for dimension table: {}", tableConfig.getTableName());
Review comment:
Same here
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -463,6 +467,36 @@ private void ensureMinReplicas(TableConfig tableConfig) {
}
}
+ private void ensureStorageQuotaConstraints(TableConfig tableConfig) {
+ // Dim tables must adhere to cluster level storage size limits
+ if (tableConfig.isDimTable()) {
+ QuotaConfig quotaConfig = tableConfig.getQuotaConfig();
+ String maxAllowedSize = this._controllerConf.getDimTableMaxSize();
+ long maxAllowedSizeInBytes = DataSizeUtils.toBytes(maxAllowedSize);
+
+ if (quotaConfig == null) {
+ // set a default storage quota
+ LOGGER.info("Assigning default storage quota for dimension table: {}", tableConfig.getTableName());
+ tableConfig.setQuotaConfig(
+ new QuotaConfig(maxAllowedSize, null));
+ } else {
+ if (quotaConfig.getStorage() == null) {
+ // set a default storage quota and keep the RPS value
+ LOGGER.info("Assigning default storage quota for dimension table: {}", tableConfig.getTableName());
+ tableConfig.setQuotaConfig(
+ new QuotaConfig(maxAllowedSize, quotaConfig.getMaxQueriesPerSecond())
+ );
+ } else {
+ if (quotaConfig.getStorageInBytes() > maxAllowedSizeInBytes) {
+ throw new PinotHelixResourceManager.InvalidTableConfigException(
+ "Invalid value for table storage quota. Max allowed is " + maxAllowedSize
Review comment:
(nit)
```suggestion
String.format("Invalid storage quota: %d, max allowed size: %d", storageInBytes, maxAllowedSize)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org