You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by as...@apache.org on 2021/11/23 05:22:59 UTC
[impala] 02/03: IMPALA-11031: Listmap.getIndex() name is misleading
This is an automated email from the ASF dual-hosted git repository.
asherman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit c3f2ecc95f217ca9b1c475c44f524bb2fbab5545
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
AuthorDate: Fri Nov 19 17:32:56 2021 +0100
IMPALA-11031: Listmap.getIndex() name is misleading
Listmap.getIndex(t) modifies the ListMap object when there is
no mapping for 't'. Hence the name of it is very misleading as
the reader wouldn't expect modifications from simple getters.
This patch renames it to getOrAddIndex().
Change-Id: I689dfb67e1a9104812489d6299ed43446d2fcae8
Reviewed-on: http://gerrit.cloudera.org:8080/18042
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java | 6 +++---
.../org/apache/impala/catalog/HdfsPartitionLocationCompressor.java | 2 +-
fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java | 2 +-
fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java | 3 ++-
fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java | 2 +-
fe/src/main/java/org/apache/impala/planner/KuduScanNode.java | 2 +-
fe/src/main/java/org/apache/impala/util/ListMap.java | 4 ++--
7 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java b/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
index 25f8aea..c92f5ef 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
@@ -141,7 +141,7 @@ public class HdfsPartition extends CatalogObjectImpl
int origHostIdx = FileBlock.getReplicaHostIdx(it, j);
boolean isCached = FileBlock.isReplicaCached(it, j);
TNetworkAddress origHost = origIndex.get(origHostIdx);
- int newHostIdx = dstIndex.getIndex(origHost);
+ int newHostIdx = dstIndex.getOrAddIndex(origHost);
it.mutateReplicaHostIdxs(j, FileBlock.makeReplicaIdx(isCached, newHostIdx));
}
}
@@ -178,7 +178,7 @@ public class HdfsPartition extends CatalogObjectImpl
if (isEc) {
fbFileBlockOffsets[blockIdx++] = FileBlock.createFbFileBlock(fbb,
loc.getOffset(), loc.getLength(),
- (short) hostIndex.getIndex(REMOTE_NETWORK_ADDRESS));
+ (short) hostIndex.getOrAddIndex(REMOTE_NETWORK_ADDRESS));
} else {
fbFileBlockOffsets[blockIdx++] =
FileBlock.createFbFileBlock(fbb, loc, hostIndex, numUnknownDiskIds);
@@ -399,7 +399,7 @@ public class HdfsPartition extends CatalogObjectImpl
// map it to the corresponding hostname from getHosts().
for (int i = 0; i < loc.getNames().length; ++i) {
TNetworkAddress networkAddress = BlockReplica.parseLocation(loc.getNames()[i]);
- short replicaIdx = (short) hostIndex.getIndex(networkAddress);
+ short replicaIdx = (short) hostIndex.getOrAddIndex(networkAddress);
boolean isReplicaCached = cachedHosts.contains(loc.getHosts()[i]);
replicaIdx = makeReplicaIdx(isReplicaCached, replicaIdx);
fbb.addShort(replicaIdx);
diff --git a/fe/src/main/java/org/apache/impala/catalog/HdfsPartitionLocationCompressor.java b/fe/src/main/java/org/apache/impala/catalog/HdfsPartitionLocationCompressor.java
index 2ba6414..f4f9bf3 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HdfsPartitionLocationCompressor.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HdfsPartitionLocationCompressor.java
@@ -75,7 +75,7 @@ public class HdfsPartitionLocationCompressor {
// Compress a location prefix, adding it to the bidirectional map (indexToPrefix_,
// prefixToIndex_) if it is not already present.
private int prefixToIndex(String s) {
- return prefixMap_.getIndex(s);
+ return prefixMap_.getOrAddIndex(s);
}
// A surrogate for THdfsPartitionLocation, which represents a partition's location
diff --git a/fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java b/fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
index 1860baa..c89b276 100644
--- a/fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
+++ b/fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
@@ -326,7 +326,7 @@ public class DataSourceScanNode extends ScanNode {
private void computeScanRangeLocations(Analyzer analyzer) {
// TODO: Does the port matter?
TNetworkAddress networkAddress = addressToTNetworkAddress("localhost:12345");
- Integer hostIndex = analyzer.getHostIndex().getIndex(networkAddress);
+ Integer hostIndex = analyzer.getHostIndex().getOrAddIndex(networkAddress);
scanRangeSpecs_ = new TScanRangeSpec();
scanRangeSpecs_.addToConcrete_ranges(new TScanRangeLocationList(
new TScanRange(), Lists.newArrayList(new TScanRangeLocation(hostIndex))));
diff --git a/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java b/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
index 999c5bf..17f570f 100644
--- a/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
+++ b/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
@@ -497,7 +497,8 @@ public class HBaseScanNode extends ScanNode {
TScanRangeLocationList scanRangeLocation = new TScanRangeLocationList();
TNetworkAddress networkAddress = addressToTNetworkAddress(locEntry.getKey());
scanRangeLocation.addToLocations(
- new TScanRangeLocation(analyzer.getHostIndex().getIndex(networkAddress)));
+ new TScanRangeLocation(
+ analyzer.getHostIndex().getOrAddIndex(networkAddress)));
TScanRange scanRange = new TScanRange();
scanRange.setHbase_key_range(keyRange);
diff --git a/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java b/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
index b9e84d5..b84ce7f 100644
--- a/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
+++ b/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
@@ -1314,7 +1314,7 @@ public class HdfsScanNode extends ScanNode {
partition.getHostIndex().getEntry(replicaHostIdx);
Preconditions.checkNotNull(networkAddress);
// Translate from network address to the global (to this request) host index.
- Integer globalHostIdx = analyzer.getHostIndex().getIndex(networkAddress);
+ Integer globalHostIdx = analyzer.getHostIndex().getOrAddIndex(networkAddress);
location.setHost_idx(globalHostIdx);
if (fsHasBlocks && !fileDesc.getIsEc() && FileBlock.getDiskId(block, j) == -1) {
++numScanRangesNoDiskIds_;
diff --git a/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java b/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
index 3eb881a..dbb49a7 100644
--- a/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
+++ b/fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
@@ -246,7 +246,7 @@ public class KuduScanNode extends ScanNode {
TNetworkAddress address =
new TNetworkAddress(replica.getRpcHost(), replica.getRpcPort());
// Use the network address to look up the host in the global list
- Integer hostIndex = analyzer.getHostIndex().getIndex(address);
+ Integer hostIndex = analyzer.getHostIndex().getOrAddIndex(address);
locations.add(new TScanRangeLocation(hostIndex));
hostIndexSet_.add(hostIndex);
}
diff --git a/fe/src/main/java/org/apache/impala/util/ListMap.java b/fe/src/main/java/org/apache/impala/util/ListMap.java
index db3942b..228a2a6 100644
--- a/fe/src/main/java/org/apache/impala/util/ListMap.java
+++ b/fe/src/main/java/org/apache/impala/util/ListMap.java
@@ -53,7 +53,7 @@ public class ListMap<T> {
* Map from T t to Integer index. If the mapping from t doesn't
* exist, then create a new mapping from t to a unique index.
*/
- public int getIndex(T t) {
+ public int getOrAddIndex(T t) {
Integer index = map_.get(t);
if (index != null) return index;
// No match was found, add a new entry.
@@ -70,7 +70,7 @@ public class ListMap<T> {
}
/**
- * Populate the bi-map from the given list. Does not perform a copy
+ * Populate the bi-map from the given list. Does not perform a copy
* of the list.
*/
public synchronized void populate(List<T> list) {