Skip to content

Commit

Permalink
[BugFix] fix expensive Partition.equals (#34961)
Browse files Browse the repository at this point in the history 
Signed-off-by: Murphy <mofei@starrocks.com>
  • Loading branch information
@murphyatwork
murphyatwork authored Nov 14, 2023
1 parent 0053957 commit 5cc830c
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 59 deletions.
8 changes: 5 additions & 3 deletions fe/fe-core/src/main/java/com/starrocks/catalog/ListPartitionInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.starrocks.analysis.LiteralExpr;
import com.starrocks.common.AnalysisException;
import com.starrocks.common.DdlException;
import com.starrocks.common.Pair;
import com.starrocks.common.io.Text;
import com.starrocks.lake.DataCacheInfo;
import com.starrocks.persist.ListPartitionPersistInfo;
Expand Down Expand Up @@ -346,15 +347,16 @@ public String getValuesFormat(long partitionId) {
return "";
}

public void handleNewListPartitionDescs(Map<Partition, PartitionDesc> partitionMap,
public void handleNewListPartitionDescs(List<Pair<Partition, PartitionDesc>> partitionList,
Set<String> existPartitionNameSet, boolean isTempPartition)
throws DdlException {
try {
for (Partition partition : partitionMap.keySet()) {
for (Pair<Partition, PartitionDesc> entry : partitionList) {
Partition partition = entry.first;
String name = partition.getName();
if (!existPartitionNameSet.contains(name)) {
long partitionId = partition.getId();
PartitionDesc partitionDesc = partitionMap.get(partition);
PartitionDesc partitionDesc = entry.second;
Preconditions.checkArgument(partitionDesc instanceof SinglePartitionDesc);
Preconditions.checkArgument(((SinglePartitionDesc) partitionDesc).isAnalyzed());
this.idToDataProperty.put(partitionId, partitionDesc.getPartitionDataProperty());
Expand Down
24 changes: 5 additions & 19 deletions fe/fe-core/src/main/java/com/starrocks/catalog/Partition.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -578,7 +577,7 @@ public void readFields(DataInput in) throws IOException {

@Override
public int hashCode() {
return Objects.hashCode(visibleVersion, baseIndex, distributionInfo);
return Objects.hashCode(id, visibleVersion, baseIndex, distributionInfo);
}

@Override
Expand All @@ -591,24 +590,11 @@ public boolean equals(Object obj) {
}

Partition partition = (Partition) obj;
if (idToVisibleRollupIndex != partition.idToVisibleRollupIndex) {
if (idToVisibleRollupIndex.size() != partition.idToVisibleRollupIndex.size()) {
return false;
}
for (Entry<Long, MaterializedIndex> entry : idToVisibleRollupIndex.entrySet()) {
long key = entry.getKey();
if (!partition.idToVisibleRollupIndex.containsKey(key)) {
return false;
}
if (!entry.getValue().equals(partition.idToVisibleRollupIndex.get(key))) {
return false;
}
}
}

return (visibleVersion == partition.visibleVersion)
return (id == partition.id)
&& (visibleVersion == partition.visibleVersion)
&& (baseIndex.equals(partition.baseIndex)
&& distributionInfo.equals(partition.distributionInfo));
&& distributionInfo.equals(partition.distributionInfo))
&& Objects.equal(idToVisibleRollupIndex, partition.idToVisibleRollupIndex);
}

@Override
Expand Down
10 changes: 6 additions & 4 deletions fe/fe-core/src/main/java/com/starrocks/catalog/RangePartitionInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.google.gson.annotations.SerializedName;
import com.starrocks.common.AnalysisException;
import com.starrocks.common.DdlException;
import com.starrocks.common.Pair;
import com.starrocks.common.util.RangeUtils;
import com.starrocks.lake.DataCacheInfo;
import com.starrocks.persist.RangePartitionPersistInfo;
Expand Down Expand Up @@ -247,17 +248,18 @@ public void createAutomaticShadowPartition(long partitionId, String replicateNum
idToStorageCacheInfo.put(partitionId, new DataCacheInfo(true, false));
}

public void handleNewRangePartitionDescs(Map<Partition, PartitionDesc> partitionMap,
public void handleNewRangePartitionDescs(List<Pair<Partition, PartitionDesc>> partitionList,
Set<String> existPartitionNameSet,
boolean isTemp) throws DdlException {
for (Partition partition : partitionMap.keySet()) {
for (Pair<Partition, PartitionDesc> entry : partitionList) {
Partition partition = entry.first;
if (!existPartitionNameSet.contains(partition.getName())) {
long partitionId = partition.getId();
SingleRangePartitionDesc desc = (SingleRangePartitionDesc) partitionMap.get(partition);
SingleRangePartitionDesc desc = (SingleRangePartitionDesc) entry.second;
Preconditions.checkArgument(desc.isAnalyzed());
Range<PartitionKey> range;
try {
range = checkAndCreateRange((SingleRangePartitionDesc) partitionMap.get(partition), isTemp);
range = checkAndCreateRange((SingleRangePartitionDesc) entry.second, isTemp);
setRangeInternal(partitionId, isTemp, range);
} catch (IllegalArgumentException e) {
// Range.closedOpen may throw this if (lower > upper)
Expand Down
20 changes: 0 additions & 20 deletions fe/fe-core/src/main/java/com/starrocks/persist/PartitionPersistInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@

package com.starrocks.persist;

import com.google.common.base.Objects;
import com.google.common.collect.Range;
import com.starrocks.catalog.DataProperty;
import com.starrocks.catalog.Partition;
Expand Down Expand Up @@ -137,23 +136,4 @@ public void readFields(DataInput in) throws IOException {
isTempPartition = in.readBoolean();
}

@Override
public int hashCode() {
return Objects.hashCode(dbId, tableId);
}

public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (!(obj instanceof PartitionPersistInfo)) {
return false;
}

PartitionPersistInfo info = (PartitionPersistInfo) obj;

return dbId.equals(info.dbId)
&& tableId.equals(info.tableId)
&& partition.equals(info.partition);
}
}
29 changes: 16 additions & 13 deletions fe/fe-core/src/main/java/com/starrocks/server/LocalMetastore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1115,13 +1115,13 @@ private void checkDataProperty(List<PartitionDesc> partitionDescs) {
}
}

private Map<Partition, PartitionDesc> createPartitionMap(Database db, OlapTable copiedTable,
private List<Pair<Partition, PartitionDesc>> createPartitionMap(Database db, OlapTable copiedTable,
List<PartitionDesc> partitionDescs,
HashMap<String, Set<Long>> partitionNameToTabletSet,
Set<Long> tabletIdSetForAll,
Set<String> existPartitionNameSet)
throws DdlException {
Map<Partition, PartitionDesc> partitionMap = Maps.newHashMap();
List<Pair<Partition, PartitionDesc>> partitionList = Lists.newArrayList();
for (PartitionDesc partitionDesc : partitionDescs) {
long partitionId = getNextId();
DataProperty dataProperty = partitionDesc.getPartitionDataProperty();
Expand All @@ -1141,11 +1141,11 @@ private Map<Partition, PartitionDesc> createPartitionMap(Database db, OlapTable
Partition partition =
createPartition(db, copiedTable, partitionId, partitionName, version, tabletIdSet);

partitionMap.put(partition, partitionDesc);
partitionList.add(Pair.create(partition, partitionDesc));
tabletIdSetForAll.addAll(tabletIdSet);
partitionNameToTabletSet.put(partitionName, tabletIdSet);
}
return partitionMap;
return partitionList;
}

private void checkIfMetaChange(OlapTable olapTable, OlapTable copiedTable, String tableName) throws DdlException {
Expand Down Expand Up @@ -1175,29 +1175,31 @@ private void checkIfMetaChange(OlapTable olapTable, OlapTable copiedTable, Strin
}
}

private void updatePartitionInfo(PartitionInfo partitionInfo, Map<Partition, PartitionDesc> partitionMap,
private void updatePartitionInfo(PartitionInfo partitionInfo, List<Pair<Partition, PartitionDesc>> partitionList,
Set<String> existPartitionNameSet, AddPartitionClause addPartitionClause,
OlapTable olapTable)
throws DdlException {
boolean isTempPartition = addPartitionClause.isTempPartition();
if (partitionInfo instanceof RangePartitionInfo) {
RangePartitionInfo rangePartitionInfo = (RangePartitionInfo) partitionInfo;
rangePartitionInfo.handleNewRangePartitionDescs(partitionMap, existPartitionNameSet, isTempPartition);
rangePartitionInfo.handleNewRangePartitionDescs(partitionList, existPartitionNameSet, isTempPartition);
} else if (partitionInfo instanceof ListPartitionInfo) {
ListPartitionInfo listPartitionInfo = (ListPartitionInfo) partitionInfo;
listPartitionInfo.handleNewListPartitionDescs(partitionMap, existPartitionNameSet, isTempPartition);
listPartitionInfo.handleNewListPartitionDescs(partitionList, existPartitionNameSet, isTempPartition);
} else {
throw new DdlException("Only support adding partition to range/list partitioned table");
}

if (isTempPartition) {
for (Partition partition : partitionMap.keySet()) {
for (Pair<Partition, PartitionDesc> entry : partitionList) {
Partition partition = entry.first;
if (!existPartitionNameSet.contains(partition.getName())) {
olapTable.addTempPartition(partition);
}
}
} else {
for (Partition partition : partitionMap.keySet()) {
for (Pair<Partition, PartitionDesc> entry : partitionList) {
Partition partition = entry.first;
if (!existPartitionNameSet.contains(partition.getName())) {
olapTable.addPartition(partition);
}
Expand Down Expand Up @@ -1395,11 +1397,12 @@ private void addPartitions(Database db, String tableName, List<PartitionDesc> pa
HashMap<String, Set<Long>> partitionNameToTabletSet = Maps.newHashMap();
try {
// create partition list
Map<Partition, PartitionDesc> partitionMap = createPartitionMap(db, copiedTable, partitionDescs,
partitionNameToTabletSet, tabletIdSetForAll, checkExistPartitionName);
List<Pair<Partition, PartitionDesc>> newPartitions =
createPartitionMap(db, copiedTable, partitionDescs, partitionNameToTabletSet, tabletIdSetForAll,
checkExistPartitionName);

// build partitions
ArrayList<Partition> partitionList = new ArrayList<>(partitionMap.keySet());
List<Partition> partitionList = newPartitions.stream().map(x -> x.first).collect(Collectors.toList());
buildPartitions(db, copiedTable, partitionList.stream().map(Partition::getSubPartitions)
.flatMap(p -> p.stream()).collect(Collectors.toList()));

Expand Down Expand Up @@ -1429,7 +1432,7 @@ private void addPartitions(Database db, String tableName, List<PartitionDesc> pa
checkPartitionType(partitionInfo);

// update partition info
updatePartitionInfo(partitionInfo, partitionMap, existPartitionNameSet, addPartitionClause, olapTable);
updatePartitionInfo(partitionInfo, newPartitions, existPartitionNameSet, addPartitionClause, olapTable);

colocateTableIndex.updateLakeTableColocationInfo(olapTable, true /* isJoin */,
null /* expectGroupId */);
Expand Down

0 comments on commit 5cc830c

Please sign in to comment.