Skip to content

Route stateless get requests to isPromotable shard#93612

Merged
Tim-Brooks merged 3 commits intoelastic:mainfrom
Tim-Brooks:custom_stateless_get_routing
Feb 9, 2023
Merged

Route stateless get requests to isPromotable shard#93612
Tim-Brooks merged 3 commits intoelastic:mainfrom
Tim-Brooks:custom_stateless_get_routing

Conversation

@Tim-Brooks
Copy link
Contributor

In stateless it is possible that search shards do not have new enough data to serve search requests. This commit works around this issue by routing those requests to promotable shards.

In stateless it is possible that search shards do not have new enough
data to serve search requests. This commit works around this issue by
routing those requests to promotable shards.
@Tim-Brooks Tim-Brooks added >non-issue :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v8.7.0 labels Feb 9, 2023
@elasticsearchmachine elasticsearchmachine added Team:Distributed Meta label for distributed team. v8.8.0 labels Feb 9, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

request.request().routing(),
request.request().preference()
);
// If it is stateless, only route isPromotableToPrimary shards
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think this comment is necessary

return new PlainShardIterator(
shards.shardId(),
shards.getShardRoutings().stream().filter(ShardRouting::isPromotableToPrimary).collect(Collectors.toList())
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be inside of getShards to avoid duplication?
Also is this a temporary solution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is temporary and I added a comment to this point. I did not move it in getShards as I felt like that surface level change was larger. And is relied on by other actions (explain and term vectors). However, I created an issue for those two transport actions, which would allow a more extensive refactoring in getShards if it makes sense.

@arteam arteam self-requested a review February 9, 2023 08:41
Copy link
Contributor

@arteam arteam left a comment

Choose a reason for hiding this comment

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

LGTM

@Tim-Brooks Tim-Brooks merged commit 9a3d2af into elastic:main Feb 9, 2023
pxsalehi added a commit that referenced this pull request Feb 28, 2023
In #93612 we introduce 
a workaround for real-time get/mget for stateless. However there 
is a null-check missing there.
pxsalehi added a commit to pxsalehi/elasticsearch that referenced this pull request Feb 28, 2023
In elastic#93612 we introduce
a workaround for real-time get/mget for stateless. However there
is a null-check missing there.
elasticsearchmachine pushed a commit that referenced this pull request Feb 28, 2023
In #93612 we introduce
a workaround for real-time get/mget for stateless. However there
is a null-check missing there.
dnhatn added a commit that referenced this pull request Mar 20, 2023
Like GET requests, we should also route term vectors requests 
to promotable copies in stateless.

Relates #93612
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Mar 20, 2023
Like GET requests, we should also route term vectors requests 
to promotable copies in stateless.

Relates elastic#93612
elasticsearchmachine pushed a commit that referenced this pull request Mar 20, 2023
Like GET requests, we should also route term vectors requests 
to promotable copies in stateless.

Relates #93612
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >non-issue Team:Distributed Meta label for distributed team. v8.7.0 v8.8.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants