Skip to content

Commit 2ec5616

Browse files
klsinceklsince
committed
cr
1 parent e391907 commit 2ec5616

File tree

3 files changed

+35
-48
lines changed

3 files changed

+35
-48
lines changed

‎pinot-segment-local/src/main/java/org/apache/pinot/segment/local/data/manager/TableDataManagerConfig.java‎

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,14 @@
2222
import com.google.common.base.Preconditions;
2323
import java.util.Collections;
2424
import java.util.HashMap;
25-
import java.util.List;
25+
import java.util.Iterator;
2626
import java.util.Map;
2727
import org.apache.commons.configuration.Configuration;
2828
import org.apache.commons.configuration.PropertiesConfiguration;
2929
import org.apache.pinot.spi.config.instance.InstanceDataManagerConfig;
3030
import org.apache.pinot.spi.config.table.SegmentsValidationAndRetentionConfig;
3131
import org.apache.pinot.spi.config.table.TableConfig;
3232
import org.apache.pinot.spi.config.table.TableType;
33-
import org.apache.pinot.spi.env.CommonsConfigurationUtils;
3433
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
3534
import org.slf4j.Logger;
3635
import org.slf4j.LoggerFactory;
@@ -50,7 +49,7 @@ public class TableDataManagerConfig {
5049
private static final String TABLE_DATA_MANAGER_AUTH = "auth";
5150

5251
private static final String TABLE_DATA_MANAGER_TIER_CONFIGS = "tierConfigs";
53-
private static final String TABLE_DATA_MANAGER_TIER_NAME = "tierName";
52+
private static final String TIER_CONFIGS_TIER_NAMES = "tierNames";
5453
private static final String TABLE_DELETED_SEGMENTS_CACHE_SIZE = "deletedSegmentsCacheSize";
5554
private static final String TABLE_DELETED_SEGMENTS_CACHE_TTL_MINUTES = "deletedSegmentsCacheTTL";
5655
private static final String TABLE_PEER_DOWNLOAD_SCHEME = "peerDownloadScheme";
@@ -100,38 +99,28 @@ public Map<String, Map<String, String>> getInstanceTierConfigs() {
10099
}
101100

102101
@VisibleForTesting
103-
static Map<String, Map<String, String>> getTierConfigMaps(Configuration tierCfgs) {
104-
List<String> cfgKeys = CommonsConfigurationUtils.getKeys(tierCfgs);
105-
Collections.sort(cfgKeys);
102+
static Map<String, Map<String, String>> getTierConfigMaps(Configuration allTierCfgs) {
103+
// Note that config names from the instance configs are all lowered cased, e.g.
104+
// - tiernames:[hotTier, coldTier]
105+
// - hottier.datadir:/tmp/multidir_test/hotTier
106+
// - coldtier.datadir:/tmp/multidir_test/coldTier
107+
// And Configuration uses ',' as the list separator to get the list of strings.
108+
// Therefore, the tier name should not contain ',' and must be unique case-insensitive.
109+
String[] tierNames = allTierCfgs.getStringArray(TIER_CONFIGS_TIER_NAMES.toLowerCase());
110+
if (tierNames == null) {
111+
LOGGER.debug("No tierConfigs from instanceConfig");
112+
return Collections.emptyMap();
113+
}
106114
Map<String, Map<String, String>> tierCfgMaps = new HashMap<>();
107-
// Collect the cfgs for each tier into the tierCfgMaps.
108-
Map<String, String> cfgMap = new HashMap<>();
109-
String lastIdx = null;
110-
String tierName = null;
111-
for (String key : cfgKeys) {
112-
String value = tierCfgs.getString(key);
113-
String cfgIdx = key.substring(0, key.indexOf('.'));
114-
if (lastIdx == null) {
115-
lastIdx = cfgIdx;
116-
} else if (!cfgIdx.equals(lastIdx)) {
117-
Preconditions.checkNotNull(tierName, "Missing tier name in instance tier configs with index: %s", lastIdx);
118-
tierCfgMaps.put(tierName, cfgMap);
119-
// Reset to collect cfgs for the next tier.
120-
cfgMap = new HashMap<>();
121-
lastIdx = cfgIdx;
122-
tierName = null;
123-
}
124-
String cfgName = key.substring(key.indexOf('.') + 1);
125-
cfgMap.put(cfgName, value);
126-
// All config names are lower cased while being passed down here.
127-
if (cfgName.equalsIgnoreCase(TABLE_DATA_MANAGER_TIER_NAME)) {
128-
tierName = value;
115+
for (String tierName : tierNames) {
116+
Configuration tierCfgs = allTierCfgs.subset(tierName.toLowerCase());
117+
for (Iterator<String> cfgKeys = tierCfgs.getKeys(); cfgKeys.hasNext(); ) {
118+
String cfgKey = cfgKeys.next();
119+
String cfgValue = tierCfgs.getString(cfgKey);
120+
tierCfgMaps.computeIfAbsent(tierName, (k) -> new HashMap<>()).put(cfgKey, cfgValue);
129121
}
130122
}
131-
if (!cfgMap.isEmpty()) {
132-
Preconditions.checkNotNull(tierName, "Missing tier name in instance tier configs with index: %s", lastIdx);
133-
tierCfgMaps.put(tierName, cfgMap);
134-
}
123+
LOGGER.debug("Got tierConfigs: {} from instanceConfig", tierCfgMaps);
135124
return tierCfgMaps;
136125
}
137126

‎pinot-segment-local/src/test/java/org/apache/pinot/segment/local/data/manager/TableDataManagerConfigTest.java‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import static org.mockito.Mockito.when;
3030
import static org.testng.Assert.assertEquals;
3131
import static org.testng.Assert.assertNull;
32+
import static org.testng.Assert.assertTrue;
3233

3334

3435
public class TableDataManagerConfigTest {
@@ -56,20 +57,18 @@ public void testOverridePeerDownloadScheme() {
5657
@Test
5758
public void testGetTierConfigMaps() {
5859
Configuration defaultConfig = new PropertiesConfiguration();
59-
defaultConfig.setProperty("tierConfigs.0.tierName", "tierX");
60-
defaultConfig.setProperty("tierConfigs.1.tierName", "tierY");
61-
defaultConfig.setProperty("tierConfigs.c.tierName", "tierZ");
62-
defaultConfig.setProperty("tierConfigs.0.dataDir", "/foo/bar");
63-
defaultConfig.setProperty("tierConfigs.1.dataDir", "/xyz/abc");
64-
defaultConfig.setProperty("tierConfigs.c.somepath", "somewhere");
6560
Map<String, Map<String, String>> tierCfgs =
6661
TableDataManagerConfig.getTierConfigMaps(defaultConfig.subset("tierConfigs"));
62+
assertTrue(tierCfgs.isEmpty());
63+
// The config names are lower cased, which is done for all instance configs.
64+
defaultConfig.setProperty("tierConfigs.tiernames", "tierX,tierY,tierZ.a.B.c");
65+
defaultConfig.setProperty("tierConfigs.tierx.datadir", "/foo/bar");
66+
defaultConfig.setProperty("tierConfigs.tiery.datadir", "/xyz/abc");
67+
defaultConfig.setProperty("tierConfigs.tierz.a.b.c.somepath", "somewhere");
68+
tierCfgs = TableDataManagerConfig.getTierConfigMaps(defaultConfig.subset("tierConfigs"));
6769
assertEquals(tierCfgs.size(), 3);
68-
assertEquals(tierCfgs.get("tierX").get("tierName"), "tierX");
69-
assertEquals(tierCfgs.get("tierX").get("dataDir"), "/foo/bar");
70-
assertEquals(tierCfgs.get("tierY").get("tierName"), "tierY");
71-
assertEquals(tierCfgs.get("tierY").get("dataDir"), "/xyz/abc");
72-
assertEquals(tierCfgs.get("tierZ").get("tierName"), "tierZ");
73-
assertEquals(tierCfgs.get("tierZ").get("somepath"), "somewhere");
70+
assertEquals(tierCfgs.get("tierX").get("datadir"), "/foo/bar");
71+
assertEquals(tierCfgs.get("tierY").get("datadir"), "/xyz/abc");
72+
assertEquals(tierCfgs.get("tierZ.a.B.c").get("somepath"), "somewhere");
7473
}
7574
}

‎pinot-tools/src/main/java/org/apache/pinot/tools/MultiDirQuickstart.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,9 @@ protected Map<String, Object> getConfigOverrides() {
5050
* "dataDir": "/tmp/multidir_test/hotTier"
5151
* }
5252
*/
53-
properties.put("pinot.server.instance.tierConfigs.0.tierName", "hotTier");
54-
properties.put("pinot.server.instance.tierConfigs.0.dataDir", "/tmp/multidir_test/hotTier");
55-
properties.put("pinot.server.instance.tierConfigs.1.tierName", "coldTier");
56-
properties.put("pinot.server.instance.tierConfigs.1.dataDir", "/tmp/multidir_test/coldTier");
53+
properties.put("pinot.server.instance.tierConfigs.tierNames", "hotTier,coldTier");
54+
properties.put("pinot.server.instance.tierConfigs.hotTier.dataDir", "/tmp/multidir_test/hotTier");
55+
properties.put("pinot.server.instance.tierConfigs.coldTier.dataDir", "/tmp/multidir_test/coldTier");
5756
return properties;
5857
}
5958

0 commit comments

Comments
 (0)