This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 52e5a2c3a6 make tableNameMap always cache list of tables (#8475)
52e5a2c3a6 is described below
commit 52e5a2c3a6d60a7201fa0d8bc5e22ff1a06287b8
Author: Rong Rong <[email protected]>
AuthorDate: Wed Apr 6 11:41:07 2022 -0700
make tableNameMap always cache list of tables (#8475)
---
.../pinot/common/config/provider/TableCache.java | 52 ++++++++++++++++------
.../pinot/controller/helix/TableCacheTest.java | 38 +++++++++++-----
2 files changed, 65 insertions(+), 25 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/config/provider/TableCache.java
b/pinot-common/src/main/java/org/apache/pinot/common/config/provider/TableCache.java
index f708d42226..1f5d286608 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/config/provider/TableCache.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/config/provider/TableCache.java
@@ -18,7 +18,6 @@
*/
package org.apache.pinot.common.config.provider;
-import com.google.common.base.Preconditions;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
@@ -66,8 +65,10 @@ public class TableCache implements PinotConfigProvider {
private static final String TABLE_CONFIG_PATH_PREFIX = "/CONFIGS/TABLE/";
private static final String SCHEMA_PARENT_PATH = "/SCHEMAS";
private static final String SCHEMA_PATH_PREFIX = "/SCHEMAS/";
- private static final String LOWER_CASE_OFFLINE_TABLE_SUFFIX = "_offline";
- private static final String LOWER_CASE_REALTIME_TABLE_SUFFIX = "_realtime";
+ private static final String OFFLINE_TABLE_SUFFIX = "_OFFLINE";
+ private static final String REALTIME_TABLE_SUFFIX = "_REALTIME";
+ private static final String LOWER_CASE_OFFLINE_TABLE_SUFFIX =
OFFLINE_TABLE_SUFFIX.toLowerCase();
+ private static final String LOWER_CASE_REALTIME_TABLE_SUFFIX =
REALTIME_TABLE_SUFFIX.toLowerCase();
// NOTE: No need to use concurrent set because it is always accessed within
the ZK change listener lock
private final Set<TableConfigChangeListener> _tableConfigChangeListeners =
new HashSet<>();
@@ -84,7 +85,7 @@ public class TableCache implements PinotConfigProvider {
private final Map<String, String> _schemaNameMap = new ConcurrentHashMap<>();
// Key is lower case table name (with or without type suffix), value is
actual table name
// For case-insensitive mode only
- private final Map<String, String> _tableNameMap;
+ private final Map<String, String> _tableNameMap = new ConcurrentHashMap<>();
private final ZkSchemaChangeListener _zkSchemaChangeListener = new
ZkSchemaChangeListener();
// Key is schema name, value is schema info
@@ -93,7 +94,6 @@ public class TableCache implements PinotConfigProvider {
public TableCache(ZkHelixPropertyStore<ZNRecord> propertyStore, boolean
caseInsensitive) {
_propertyStore = propertyStore;
_caseInsensitive = caseInsensitive;
- _tableNameMap = caseInsensitive ? new ConcurrentHashMap<>() : null;
synchronized (_zkTableConfigChangeListener) {
// Subscribe child changes before reading the data to avoid missing
changes
@@ -134,18 +134,29 @@ public class TableCache implements PinotConfigProvider {
}
/**
- * For case-insensitive only, returns the actual table name for the given
case-insensitive table name (with or without
- * type suffix), or {@code null} if the table does not exist.
+ * Returns the actual table name for the given table name (with or without
type suffix), or {@code null} if the table
+ * does not exist.
*/
@Nullable
- public String getActualTableName(String caseInsensitiveTableName) {
- Preconditions.checkState(_caseInsensitive, "TableCache is not
case-insensitive");
- return _tableNameMap.get(caseInsensitiveTableName.toLowerCase());
+ public String getActualTableName(String tableName) {
+ if (_caseInsensitive) {
+ return _tableNameMap.get(tableName.toLowerCase());
+ } else {
+ return _tableNameMap.get(tableName);
+ }
+ }
+
+ /**
+ * Returns a map from table name to actual table name. For case-insensitive
case, the keys of the map are in lower
+ * case.
+ */
+ public Map<String, String> getTableNameMap() {
+ return _tableNameMap;
}
/**
- * For case-insensitive only, returns a map from lower case column name to
actual column name for the given table, or
- * {@code null} if the table schema does not exist.
+ * Returns a map from column name to actual column name for the given table,
or {@code null} if the table schema does
+ * not exist. For case-insensitive case, the keys of the map are in lower
case.
*/
@Nullable
public Map<String, String> getColumnNameMap(String rawTableName) {
@@ -241,17 +252,21 @@ public class TableCache implements PinotConfigProvider {
if (_caseInsensitive) {
_tableNameMap.put(tableNameWithType.toLowerCase(), tableNameWithType);
_tableNameMap.put(rawTableName.toLowerCase(), rawTableName);
+ } else {
+ _tableNameMap.put(tableNameWithType, tableNameWithType);
+ _tableNameMap.put(rawTableName, rawTableName);
}
}
private void removeTableConfig(String path) {
_propertyStore.unsubscribeDataChanges(path, _zkTableConfigChangeListener);
String tableNameWithType =
path.substring(TABLE_CONFIG_PATH_PREFIX.length());
+ String rawTableName =
TableNameBuilder.extractRawTableName(tableNameWithType);
_tableConfigInfoMap.remove(tableNameWithType);
removeSchemaName(tableNameWithType);
if (_caseInsensitive) {
_tableNameMap.remove(tableNameWithType.toLowerCase());
- String lowerCaseRawTableName =
TableNameBuilder.extractRawTableName(tableNameWithType).toLowerCase();
+ String lowerCaseRawTableName = rawTableName.toLowerCase();
if (TableNameBuilder.isOfflineTableResource(tableNameWithType)) {
if (!_tableNameMap.containsKey(lowerCaseRawTableName +
LOWER_CASE_REALTIME_TABLE_SUFFIX)) {
_tableNameMap.remove(lowerCaseRawTableName);
@@ -261,6 +276,17 @@ public class TableCache implements PinotConfigProvider {
_tableNameMap.remove(lowerCaseRawTableName);
}
}
+ } else {
+ _tableNameMap.remove(tableNameWithType);
+ if (TableNameBuilder.isOfflineTableResource(tableNameWithType)) {
+ if (!_tableNameMap.containsKey(rawTableName + REALTIME_TABLE_SUFFIX)) {
+ _tableNameMap.remove(rawTableName);
+ }
+ } else {
+ if (!_tableNameMap.containsKey(rawTableName + OFFLINE_TABLE_SUFFIX)) {
+ _tableNameMap.remove(rawTableName);
+ }
+ }
}
}
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/TableCacheTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/TableCacheTest.java
index 874d993604..b85fd2c106 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/TableCacheTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/TableCacheTest.java
@@ -36,6 +36,7 @@ import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.apache.pinot.util.TestUtils;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import static org.testng.Assert.*;
@@ -56,10 +57,10 @@ public class TableCacheTest {
ControllerTestUtils.setupClusterAndValidate();
}
- @Test
- public void testTableCache()
+ @Test(dataProvider = "testTableCacheDataProvider")
+ public void testTableCache(boolean isCaseInsensitive)
throws Exception {
- TableCache tableCache = new
TableCache(ControllerTestUtils.getPropertyStore(), true);
+ TableCache tableCache = new
TableCache(ControllerTestUtils.getPropertyStore(), isCaseInsensitive);
assertNull(tableCache.getSchema(SCHEMA_NAME));
assertNull(tableCache.getColumnNameMap(SCHEMA_NAME));
@@ -83,10 +84,10 @@ public class TableCacheTest {
.addSingleValueDimension(BuiltInVirtualColumn.HOSTNAME,
DataType.STRING)
.addSingleValueDimension(BuiltInVirtualColumn.SEGMENTNAME,
DataType.STRING).build();
Map<String, String> expectedColumnMap = new HashMap<>();
- expectedColumnMap.put("testcolumn", "testColumn");
- expectedColumnMap.put("$docid", "$docId");
- expectedColumnMap.put("$hostname", "$hostName");
- expectedColumnMap.put("$segmentname", "$segmentName");
+ expectedColumnMap.put(isCaseInsensitive ? "testcolumn" : "testColumn",
"testColumn");
+ expectedColumnMap.put(isCaseInsensitive ? "$docid" : "$docId", "$docId");
+ expectedColumnMap.put(isCaseInsensitive ? "$hostname" : "$hostName",
"$hostName");
+ expectedColumnMap.put(isCaseInsensitive ? "$segmentname" : "$segmentName",
"$segmentName");
assertEquals(tableCache.getSchema(SCHEMA_NAME), expectedSchema);
assertEquals(tableCache.getColumnNameMap(SCHEMA_NAME), expectedColumnMap);
assertNull(tableCache.getSchema(RAW_TABLE_NAME));
@@ -101,9 +102,10 @@ public class TableCacheTest {
// Wait for at most 10 seconds for the callback to add the table config to
the cache
TestUtils.waitForCondition(
aVoid ->
tableConfig.equals(tableCache.getTableConfig(OFFLINE_TABLE_NAME)) &&
RAW_TABLE_NAME.equals(
- tableCache.getActualTableName(MANGLED_RAW_TABLE_NAME)) &&
OFFLINE_TABLE_NAME.equals(
- tableCache.getActualTableName(MANGLED_OFFLINE_TABLE_NAME)),
10_000L,
+ tableCache.getActualTableName(RAW_TABLE_NAME)) &&
OFFLINE_TABLE_NAME.equals(
+ tableCache.getActualTableName(OFFLINE_TABLE_NAME)), 10_000L,
"Failed to add the table config to the cache");
+ // It should only add OFFLINE and normal table.
assertNull(tableCache.getActualTableName(REALTIME_TABLE_NAME));
// Schema can be accessed by both the schema name and the raw table name
assertEquals(tableCache.getSchema(SCHEMA_NAME), expectedSchema);
@@ -134,7 +136,7 @@ public class TableCacheTest {
// - Verify if the callback is fully done by checking the schema change
lister because it is the last step of the
// callback handling
expectedSchema.addField(new DimensionFieldSpec("newColumn", DataType.LONG,
true));
- expectedColumnMap.put("newcolumn", "newColumn");
+ expectedColumnMap.put(isCaseInsensitive ? "newcolumn" : "newColumn",
"newColumn");
TestUtils.waitForCondition(aVoid -> {
assertNotNull(tableCache.getSchema(SCHEMA_NAME));
assertEquals(schemaChangeListener._schemaList.size(), 1);
@@ -165,8 +167,15 @@ public class TableCacheTest {
assertEquals(tableCache.getTableConfig(OFFLINE_TABLE_NAME), tableConfig);
assertNull(tableCache.getSchema(RAW_TABLE_NAME));
assertNull(tableCache.getColumnNameMap(RAW_TABLE_NAME));
- assertEquals(tableCache.getActualTableName(MANGLED_RAW_TABLE_NAME),
RAW_TABLE_NAME);
- assertEquals(tableCache.getActualTableName(MANGLED_OFFLINE_TABLE_NAME),
OFFLINE_TABLE_NAME);
+ if (isCaseInsensitive) {
+ assertEquals(tableCache.getActualTableName(MANGLED_RAW_TABLE_NAME),
RAW_TABLE_NAME);
+ assertEquals(tableCache.getActualTableName(MANGLED_OFFLINE_TABLE_NAME),
OFFLINE_TABLE_NAME);
+ } else {
+ assertNull(tableCache.getActualTableName(MANGLED_RAW_TABLE_NAME));
+ assertNull(tableCache.getActualTableName(MANGLED_OFFLINE_TABLE_NAME));
+ assertEquals(tableCache.getActualTableName(RAW_TABLE_NAME),
RAW_TABLE_NAME);
+ assertEquals(tableCache.getActualTableName(OFFLINE_TABLE_NAME),
OFFLINE_TABLE_NAME);
+ }
assertNull(tableCache.getActualTableName(REALTIME_TABLE_NAME));
assertEquals(tableCache.getSchema(SCHEMA_NAME), expectedSchema);
assertEquals(tableCache.getColumnNameMap(SCHEMA_NAME), expectedColumnMap);
@@ -202,6 +211,11 @@ public class TableCacheTest {
assertEquals(tableConfigChangeListener._tableConfigList.size(), 0);
}
+ @DataProvider(name = "testTableCacheDataProvider")
+ public Object[][] provideCaseInsensitiveSetting() {
+ return new Object[][]{new Object[]{true}, new Object[]{false}};
+ }
+
private static class TestTableConfigChangeListener implements
TableConfigChangeListener {
private volatile List<TableConfig> _tableConfigList;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
