Stateless real-time GET #93976

pxsalehi · 2023-02-21T13:39:41Z

For real-time get on Stateless, we'd need to first check the indexing shard whether it has
the document in its Translog, if not we might have to wait on the search shard and then
handle the GET locally.

Relates ES-5537

pxsalehi · 2023-02-22T14:40:53Z

Thanks for the feedback @henningandersen.

I think there is no way around contacting the indexing shard for real-time GET.

So as long as the GET request has its realtime field set to true (which is the default) we'd forward it to the indexing shard?

pxsalehi · 2023-02-22T16:14:37Z

So as long as the GET request has its realtime field set to true (which is the default) we'd forward it to the indexing shard?

We discussed this on another channel with Henning. The disadvantage here would be that this might cause a lot of load on the indexing shards. We'd rather only contact the indexing shards for a Translog lookup.

.../src/main/java/org/elasticsearch/action/support/single/shard/TransportSingleShardAction.java

server/src/main/java/org/elasticsearch/action/get/TransportGetAction.java

server/src/main/java/org/elasticsearch/action/get/TransportShardMultiGetAction.java

elasticsearchmachine · 2023-03-06T10:59:41Z

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

rest-api-spec/src/yamlRestTest/java/org/elasticsearch/test/rest/ClientYamlTestSuiteIT.java

pxsalehi · 2023-03-07T08:41:17Z

@elasticmachine update branch

pxsalehi · 2023-03-07T08:50:02Z

@elasticmachine update branch

pxsalehi · 2023-03-08T09:38:16Z

This is ready for review.

This is a change broken off from the ongoing real-time GET PR (#93976). It is just an action that can be used to invoke the new `ShardGetService.getFromTranslog` in #95736. It will be used on the search shards as a first step to handle a real-time get. Relates #93976, ES-5537

pxsalehi · 2023-05-23T14:38:03Z

@elasticmachine test this please

hmm... seems like an infra issue!

henningandersen

LGTM.

server/src/main/java/org/elasticsearch/action/get/TransportGetAction.java

henningandersen · 2023-05-23T19:23:55Z

server/src/main/java/org/elasticsearch/action/get/TransportGetAction.java

@@ -91,6 +110,16 @@ protected void resolveRequest(ClusterState state, InternalRequest request) {
    protected void asyncShardOperation(GetRequest request, ShardId shardId, ActionListener<GetResponse> listener) throws IOException {
        IndexService indexService = indicesService.indexServiceSafe(shardId.getIndex());
        IndexShard indexShard = indexService.getShard(shardId.id());
+        if (indexShard.routingEntry() == null) {


Can this happen? Looks like we init it in IndexShard constructor and then only update it to non-null values?

I'm not entirely sure. I vaguely remember running into an issue in the begging when I was working on this, and adding this helped. Maybe the issue was something else. I do see that similar checks are done in a couple of other transport actions, e.g., TransportIndicesStatsAction and DataStreamsStatsTransportAction.

Yeah, but that looks like both comes from TransportIndicesStatsAction, which was done in 2014 and thus on a very different code base. I think we should assume this does not happen, since we assume so in other actions, for instance TransportGetFromTranslogAction, PostWriteRefresh and many others. Unless you know where it can happen I suggest to remove the new null check, we do not want null-checks all over for things that cannot be null.

henningandersen · 2023-05-23T19:37:10Z

server/src/main/java/org/elasticsearch/action/get/TransportGetAction.java

+        if (shardRoutingTable.primaryShard() == null || shardRoutingTable.primaryShard().active() == false) {
+            throw new NoShardAvailableActionException(shardId, "primary shard is not active");
+        }
+        DiscoveryNode node = clusterService.state().nodes().get(shardRoutingTable.primaryShard().currentNodeId());


Can we use the same ClusterState instance here rather than asking for it again? That way, the null check below can turn into an assertion (assert node != null).

pxsalehi · 2023-05-24T15:07:54Z

@elasticmachine update branch

pxsalehi · 2023-05-25T08:39:41Z

@elasticmachine update branch

The mget counterpart of #93976. Relates ES-5677

pxsalehi added WIP :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. labels Feb 21, 2023

elasticsearchmachine added the v8.8.0 label Feb 21, 2023

pxsalehi changed the title ~~Defer real-time GET to unpromotable shard copies if necessary~~ Defer real-time GET to promotable shard copies if necessary Feb 21, 2023

pxsalehi added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Feb 21, 2023

pxsalehi requested review from DaveCTurner, tlrx and Tim-Brooks February 21, 2023 15:26

pxsalehi added WIP and removed WIP labels Feb 21, 2023

This comment was marked as outdated.

Sign in to view

idegtiarenko reviewed Feb 22, 2023

View reviewed changes

.../src/main/java/org/elasticsearch/action/support/single/shard/TransportSingleShardAction.java Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

idegtiarenko reviewed Feb 23, 2023

View reviewed changes

server/src/main/java/org/elasticsearch/action/get/TransportGetAction.java Show resolved Hide resolved

idegtiarenko reviewed Feb 23, 2023

View reviewed changes

server/src/main/java/org/elasticsearch/action/get/TransportShardMultiGetAction.java Outdated Show resolved Hide resolved

pxsalehi mentioned this pull request Mar 3, 2023

Use 1 replica in YAML tests that issue get/exist calls #94303

Merged

pxsalehi changed the title ~~Defer real-time GET to promotable shard copies if necessary~~ Stateless real-time GET Mar 3, 2023

pxsalehi removed the WIP label Mar 6, 2023

pxsalehi marked this pull request as ready for review March 6, 2023 10:59

pxsalehi added the >non-issue label Mar 6, 2023

pxsalehi commented Mar 6, 2023

View reviewed changes

rest-api-spec/src/yamlRestTest/java/org/elasticsearch/test/rest/ClientYamlTestSuiteIT.java Outdated Show resolved Hide resolved

pxsalehi requested review from henningandersen and removed request for DaveCTurner March 6, 2023 13:10

pxsalehi requested a review from idegtiarenko March 8, 2023 09:36

gmarouli added the v8.9.0 label Apr 26, 2023

pxsalehi mentioned this pull request May 10, 2023

Introduce TransportGetFromTranslogAction #95998

Merged

pxsalehi force-pushed the ps230220-realTimeGet branch from f53961b to 943a811 Compare May 23, 2023 14:27

real time get

6367839

pxsalehi force-pushed the ps230220-realTimeGet branch from 943a811 to 6367839 Compare May 23, 2023 16:37

pxsalehi requested review from tlrx and henningandersen and removed request for tlrx, idegtiarenko, Tim-Brooks and henningandersen May 23, 2023 19:11

henningandersen approved these changes May 23, 2023

View reviewed changes

address comments

bd976be

pxsalehi added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) and removed auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels May 24, 2023

add comment

59feef3

Merge branch 'main' into ps230220-realTimeGet

3f3a8f6

pxsalehi added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) and removed auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels May 24, 2023

Merge branch 'main' into ps230220-realTimeGet

cb62cd4

pxsalehi added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) and removed auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels May 25, 2023

pxsalehi merged commit 1762733 into elastic:main May 25, 2023

pxsalehi mentioned this pull request Jun 12, 2023

Stateless real-time mget #96763

Merged

elasticsearchmachine pushed a commit that referenced this pull request Jun 15, 2023

Stateless real-time mget (#96763)

01731ae

The mget counterpart of #93976. Relates ES-5677

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stateless real-time GET #93976

Stateless real-time GET #93976

pxsalehi commented Feb 21, 2023 •

edited

Loading

This comment was marked as outdated.

pxsalehi commented Feb 22, 2023

pxsalehi commented Feb 22, 2023

This comment was marked as outdated.

elasticsearchmachine commented Mar 6, 2023

pxsalehi commented Mar 7, 2023

pxsalehi commented Mar 7, 2023

pxsalehi commented Mar 8, 2023

pxsalehi commented May 23, 2023

henningandersen left a comment

henningandersen May 23, 2023

pxsalehi May 24, 2023

henningandersen May 24, 2023

pxsalehi May 24, 2023

henningandersen May 23, 2023

pxsalehi commented May 24, 2023

pxsalehi commented May 25, 2023

Stateless real-time GET #93976

Stateless real-time GET #93976

Conversation

pxsalehi commented Feb 21, 2023 • edited Loading

This comment was marked as outdated.

pxsalehi commented Feb 22, 2023

pxsalehi commented Feb 22, 2023

This comment was marked as outdated.

elasticsearchmachine commented Mar 6, 2023

pxsalehi commented Mar 7, 2023

pxsalehi commented Mar 7, 2023

pxsalehi commented Mar 8, 2023

pxsalehi commented May 23, 2023

henningandersen left a comment

Choose a reason for hiding this comment

henningandersen May 23, 2023

Choose a reason for hiding this comment

pxsalehi May 24, 2023

Choose a reason for hiding this comment

henningandersen May 24, 2023

Choose a reason for hiding this comment

pxsalehi May 24, 2023

Choose a reason for hiding this comment

henningandersen May 23, 2023

Choose a reason for hiding this comment

pxsalehi commented May 24, 2023

pxsalehi commented May 25, 2023

pxsalehi commented Feb 21, 2023 •

edited

Loading