Skip to content
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2000,17 +2000,13 @@ public Builder numberOfShards(int numberOfShards) {
* Builder to create IndexMetadata that has an increased shard count (used for re-shard).
* The new shard count must be a multiple of the original shardcount.
* We do not support shrinking the shard count.
* @param shardCount updated shardCount
*
* TODO: Check if this.version needs to be incremented
* @param targetShardCount target shard count after resharding
*/
public Builder reshardAddShards(int shardCount) {
// Assert routingNumShards is null ?
// Assert numberOfShards > 0
if (shardCount % numberOfShards() != 0) {
public Builder reshardAddShards(int targetShardCount, final int sourceNumShards) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't sourceNumShards extractable from this.settings? Passing it in opens the door to the same kind of inconsistency I was worried about before when we were passing in the whole metadata.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I think I might have finally got it right this time !

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Sorry about all the iteration.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a simple unittest of this functionality?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added an IT test to StatelessReshardIT in the linked stateless PR. Were you thinking of something different ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be nice as a small followup to have coverage in the same repo as the code (just in unit test form), instead of relying on an external repository, since they don't always change in lockstep.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a follow up ticket for this.

if (targetShardCount % numberOfShards() != 0) {
throw new IllegalArgumentException(
"New shard count ["
+ shardCount
+ targetShardCount
+ "] should be a multiple"
+ " of current shard count ["
+ numberOfShards()
Expand All @@ -2019,13 +2015,12 @@ public Builder reshardAddShards(int shardCount) {
+ "]"
);
}
IndexVersion indexVersionCreated = indexCreatedVersion(settings);
settings = Settings.builder().put(settings).put(SETTING_NUMBER_OF_SHARDS, shardCount).build();
var newPrimaryTerms = new long[shardCount];
settings = Settings.builder().put(settings).put(SETTING_NUMBER_OF_SHARDS, targetShardCount).build();
var newPrimaryTerms = new long[targetShardCount];
Arrays.fill(newPrimaryTerms, this.primaryTerms.length, newPrimaryTerms.length, SequenceNumbers.UNASSIGNED_PRIMARY_TERM);
System.arraycopy(primaryTerms, 0, newPrimaryTerms, 0, this.primaryTerms.length);
primaryTerms = newPrimaryTerms;
routingNumShards = MetadataCreateIndexService.calculateNumRoutingShards(shardCount, indexVersionCreated);
routingNumShards = MetadataCreateIndexService.getIndexNumberOfRoutingShards(settings, sourceNumShards, this.routingNumShards);
return this;
}

Expand Down Expand Up @@ -3034,7 +3029,7 @@ public static ShardId selectCloneShard(int shardId, IndexMetadata sourceIndexMet
return new ShardId(sourceIndexMetadata.getIndex(), shardId);
}

private static void assertSplitMetadata(int numSourceShards, int numTargetShards, IndexMetadata sourceIndexMetadata) {
public static void assertSplitMetadata(int numSourceShards, int numTargetShards, IndexMetadata sourceIndexMetadata) {
if (numSourceShards > numTargetShards) {
throw new IllegalArgumentException(
"the number of source shards ["
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1282,10 +1282,23 @@ private static void validateSoftDeleteSettings(Settings indexSettings) {
* it will return the value configured for that index.
*/
static int getIndexNumberOfRoutingShards(Settings indexSettings, @Nullable IndexMetadata sourceMetadata) {
final int routingNumShards = getIndexNumberOfRoutingShards(
indexSettings,
sourceMetadata == null ? 1 : sourceMetadata.getNumberOfShards(),
sourceMetadata == null ? 0 : sourceMetadata.getRoutingNumShards()
);
return routingNumShards;
}

/**
* Calculates the number of routing shards based on the configured value in indexSettings or if recovering from another index
* it will return the value configured for that index.
*/
static int getIndexNumberOfRoutingShards(Settings indexSettings, final int sourceNumShards, final int sourceRoutingNumShards) {
final int numTargetShards = INDEX_NUMBER_OF_SHARDS_SETTING.get(indexSettings);
final IndexVersion indexVersionCreated = IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(indexSettings);
final int routingNumShards;
if (sourceMetadata == null || sourceMetadata.getNumberOfShards() == 1) {
if (sourceNumShards == 1) {
// in this case we either have no index to recover from or
// we have a source index with 1 shard and without an explicit split factor
// or one that is valid in that case we can split into whatever and auto-generate a new factor.
Expand All @@ -1299,7 +1312,7 @@ static int getIndexNumberOfRoutingShards(Settings indexSettings, @Nullable Index
} else {
assert IndexMetadata.INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING.exists(indexSettings) == false
: "index.number_of_routing_shards should not be present on the target index on resize";
routingNumShards = sourceMetadata.getRoutingNumShards();
routingNumShards = sourceRoutingNumShards;
}
return routingNumShards;
}
Expand Down