Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Only send setting delta in AlterTableRequest #15962

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+439 −621
Draft

Only send setting delta in AlterTableRequest #15962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/src/main/java/io/crate/analyze/AnalyzedCreateTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public BoundCreateTable bind(NumberOfShards numberOfShards,
}

TableProperties.analyze(
tableParameter, TableParameters.TABLE_CREATE_PARAMETER_INFO, properties.map(toValue), true);
tableParameter, TableParameters.TABLE_CREATE_PARAMETER_INFO, properties.map(toValue));

Optional<ColumnIdent> optClusteredBy = clusteredBy
.flatMap(ClusteredBy::column)
Expand Down
48 changes: 6 additions & 42 deletions server/src/main/java/io/crate/analyze/BoundAlterTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,49 +21,13 @@

package io.crate.analyze;

import io.crate.metadata.PartitionName;
import io.crate.metadata.table.TableInfo;

import org.jetbrains.annotations.Nullable;
import java.util.Optional;

public class BoundAlterTable {

private final TableInfo tableInfo;
private final PartitionName partitionName;
private final TableParameter tableParameter;
private final boolean excludePartitions;
private final boolean partitioned;

public BoundAlterTable(TableInfo tableInfo,
@Nullable PartitionName partitionName,
TableParameter tableParameter,
boolean excludePartitions,
boolean partitioned) {
this.tableInfo = tableInfo;
this.partitionName = partitionName;
this.tableParameter = tableParameter;
this.excludePartitions = excludePartitions;
this.partitioned = partitioned;
}

public TableInfo table() {
return tableInfo;
}

public Optional<PartitionName> partitionName() {
return Optional.ofNullable(partitionName);
}

public boolean excludePartitions() {
return excludePartitions;
}

public TableParameter tableParameter() {
return tableParameter;
}
import io.crate.metadata.PartitionName;
import io.crate.sql.tree.AlterTable;

public boolean isPartitioned() {
return partitioned;
}
public record BoundAlterTable(AnalyzedAlterTable analyzedAlterTable,
AlterTable<Object> boundAlterTable,
boolean isPartitioned,
@Nullable PartitionName partitionName) {
}
22 changes: 0 additions & 22 deletions server/src/main/java/io/crate/analyze/TableParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,36 +154,14 @@ public class TableParameters {

private static final Set<Setting<?>> EXCLUDED_SETTING_FOR_METADATA_IMPORT = Set.of(NUMBER_OF_REPLICAS);

private static final Map<String, Setting<?>> SUPPORTED_SETTINGS_INCL_SHARDS
= MapBuilder.newMapBuilder(SUPPORTED_NON_FINAL_SETTINGS_DEFAULT)
.put(
stripIndexPrefix(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey()),
IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING
).immutableMap();

private static final Map<String, Setting<?>> SUPPORTED_MAPPINGS_DEFAULT = Map.of("column_policy", COLUMN_POLICY);

private static final Map<String, Setting<?>> SUPPORTED_SETTINGS_FOR_REPLICATED_TABLES = SUPPORTED_SETTINGS
.stream()
.filter(Setting::isReplicatedIndexScope)
.filter(s -> s.isFinal() == false)
.collect(Collectors.toMap(s -> stripDotSuffix(stripIndexPrefix(s.getKey())), s -> s));

public static final TableParameters TABLE_CREATE_PARAMETER_INFO
= new TableParameters(SUPPORTED_SETTINGS_DEFAULT, SUPPORTED_MAPPINGS_DEFAULT);

public static final TableParameters REPLICATED_TABLE_ALTER_PARAMETER_INFO
= new TableParameters(SUPPORTED_SETTINGS_FOR_REPLICATED_TABLES, Map.of());

public static final TableParameters TABLE_ALTER_PARAMETER_INFO
= new TableParameters(SUPPORTED_SETTINGS_INCL_SHARDS, SUPPORTED_MAPPINGS_DEFAULT);

public static final TableParameters PARTITIONED_TABLE_PARAMETER_INFO_FOR_TEMPLATE_UPDATE
= new TableParameters(SUPPORTED_NON_FINAL_SETTINGS_DEFAULT, Map.of());

public static final TableParameters PARTITION_PARAMETER_INFO
= new TableParameters(SUPPORTED_SETTINGS_INCL_SHARDS, Map.of());

public static final TableParameters CREATE_BLOB_TABLE_PARAMETERS = new TableParameters(
Map.of(
stripIndexPrefix(NUMBER_OF_REPLICAS.getKey()), NUMBER_OF_REPLICAS,
Expand Down
10 changes: 2 additions & 8 deletions server/src/main/java/io/crate/analyze/TableProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,37 +43,31 @@ private TableProperties() {

public static void analyze(TableParameter tableParameter,
TableParameters tableParameters,
GenericProperties<Object> properties,
boolean withDefaults) {
GenericProperties<Object> properties) {
Map<String, Setting<?>> settingMap = tableParameters.supportedSettings();
Map<String, Setting<?>> mappingsMap = tableParameters.supportedMappings();

settingsFromProperties(
tableParameter.settingsBuilder(),
properties,
settingMap,
withDefaults,
mappingsMap::containsKey,
INVALID_MESSAGE);

settingsFromProperties(
tableParameter.mappingsBuilder(),
properties,
mappingsMap,
withDefaults,
settingMap::containsKey,
INVALID_MESSAGE);
}

private static void settingsFromProperties(Settings.Builder builder,
GenericProperties<Object> properties,
Map<String, Setting<?>> supportedSettings,
boolean setDefaults,
Predicate<String> ignoreProperty,
String invalidMessage) {
if (setDefaults) {
setDefaults(builder, supportedSettings);
}
setDefaults(builder, supportedSettings);
for (Map.Entry<String, Object> entry : properties) {
String settingName = entry.getKey();
if (ignoreProperty.test(settingName)) {
Expand Down
67 changes: 32 additions & 35 deletions server/src/main/java/io/crate/execution/ddl/tables/AlterTableOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,11 @@
import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_BLOCKS_WRITE_SETTING;
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.stream.Collectors;

Expand All @@ -50,13 +48,12 @@
import org.elasticsearch.common.settings.IndexScopedSettings;
import org.elasticsearch.common.settings.Settings;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.VisibleForTesting;

import io.crate.action.FutureActionListener;
import io.crate.action.sql.CollectingResultReceiver;
import io.crate.action.sql.Sessions;
import io.crate.analyze.AnalyzedAlterTableRenameTable;
import io.crate.analyze.BoundAlterTable;
import org.jetbrains.annotations.VisibleForTesting;
import io.crate.data.Row;
import io.crate.execution.ddl.index.SwapAndDropIndexRequest;
import io.crate.execution.ddl.index.TransportSwapAndDropIndexNameAction;
Expand All @@ -68,6 +65,8 @@
import io.crate.metadata.table.TableInfo;
import io.crate.replication.logical.LogicalReplicationService;
import io.crate.replication.logical.metadata.Publication;
import io.crate.sql.tree.GenericProperties;
import io.crate.types.DataTypes;

@Singleton
public class AlterTableOperation {
Expand Down Expand Up @@ -180,51 +179,49 @@ public CompletableFuture<Long> executeAlterTableOpenClose(RelationName relationN
}

public CompletableFuture<Long> executeAlterTable(BoundAlterTable analysis) {
validateSettingsForPublishedTables(analysis.table().ident(),
analysis.tableParameter().settings(),
logicalReplicationService.publications(),
indexScopedSettings);
TableInfo table = analysis.analyzedAlterTable().tableInfo();
Settings settings = Settings.builder()
.put(analysis.boundAlterTable().genericProperties())
.build();

final Settings settings = analysis.tableParameter().settings();
final boolean includesNumberOfShardsSetting = settings.hasValue(SETTING_NUMBER_OF_SHARDS);
validateSettingsForPublishedTables(
table.ident(),
settings,
logicalReplicationService.publications(),
indexScopedSettings
);

final boolean includesNumberOfShardsSetting = settings.hasValue("number_of_shards");
final boolean isResizeOperationRequired = includesNumberOfShardsSetting &&
(!analysis.isPartitioned() || analysis.partitionName().isPresent());
(!analysis.isPartitioned() || analysis.partitionName() != null);

if (isResizeOperationRequired) {
if (settings.size() > 1) {
throw new IllegalArgumentException("Setting [number_of_shards] cannot be combined with other settings");
}
return executeAlterTableChangeNumberOfShards(analysis);
}
return executeAlterTableSetOrReset(analysis);
}

private CompletableFuture<Long> executeAlterTableSetOrReset(BoundAlterTable analysis) {
try {
AlterTableRequest request = new AlterTableRequest(
analysis.table().ident(),
analysis.partitionName().map(PartitionName::asIndexName).orElse(null),
analysis.isPartitioned(),
analysis.excludePartitions(),
analysis.tableParameter().settings(),
analysis.tableParameter().mappings()
);
return transportAlterTableAction.execute(request, r -> -1L);
} catch (IOException e) {
return FutureActionListener.failedFuture(e);
}
AlterTableRequest request = new AlterTableRequest(
table.ident(),
analysis.partitionName(),
analysis.isPartitioned(),
analysis.boundAlterTable().table().excludePartitions(),
settings,
analysis.boundAlterTable().resetProperties()
);
return transportAlterTableAction.execute(request, r -> -1L);
}

private CompletableFuture<Long> executeAlterTableChangeNumberOfShards(BoundAlterTable analysis) {
final TableInfo table = analysis.table();
final TableInfo table = analysis.analyzedAlterTable().tableInfo();
final boolean isPartitioned = analysis.isPartitioned();
String sourceIndexName;
String sourceIndexAlias;
if (isPartitioned) {
Optional<PartitionName> partitionName = analysis.partitionName();
assert partitionName.isPresent() : "Resizing operations for partitioned tables " +
PartitionName partitionName = analysis.partitionName();
assert partitionName != null : "Resizing operations for partitioned tables " +
"are only supported at partition level";
sourceIndexName = partitionName.get().asIndexName();
sourceIndexName = partitionName.asIndexName();
sourceIndexAlias = table.ident().indexNameOrAlias();
} else {
sourceIndexName = table.ident().indexNameOrAlias();
Expand All @@ -233,7 +230,7 @@ private CompletableFuture<Long> executeAlterTableChangeNumberOfShards(BoundAlter

final ClusterState currentState = clusterService.state();
final IndexMetadata sourceIndexMetadata = currentState.metadata().index(sourceIndexName);
final int targetNumberOfShards = getNumberOfShards(analysis.tableParameter().settings());
final int targetNumberOfShards = getNumberOfShards(analysis.boundAlterTable().genericProperties());
validateForResizeRequest(sourceIndexMetadata, targetNumberOfShards);

final List<ChainableAction<Long>> actions = new ArrayList<>();
Expand Down Expand Up @@ -281,9 +278,9 @@ private static void validateForResizeRequest(IndexMetadata sourceIndex, int targ
}

@VisibleForTesting
static int getNumberOfShards(final Settings settings) {
static int getNumberOfShards(GenericProperties<Object> properties) {
return Objects.requireNonNull(
settings.getAsInt(SETTING_NUMBER_OF_SHARDS, null),
DataTypes.INTEGER.implicitCast(properties.get("number_of_shards")),
"Setting 'number_of_shards' is missing"
);
}
Expand Down