When getting a CompletedReindexResponse, I'm receiving an interesting message: Invalid NEST response built from a null ApiCall which is highly exceptional, please open a bug if you see this

Elastic Stack Elasticsearch
1

Hi all,

I'm exploring the NEST client currently and followed the code for ReindexOnServer here:
https://www.elastic.co/guide/en/elasticsearch/client/net-api/current/reindexing-documents.html

I have followed the code pretty exactly, to not wait for completion, to pick up the task and check for completion in a loop:

         var reindexResponse = Client.ReindexOnServer(r => r
                .Source(s => s
                    .Index(oldId)
                )
                .Destination(d => d
                    .Index(this.UniqueId)
                )
                .WaitForCompletion(false)
);

                var taskId = reindexResponse.Task;
                var taskResponse = Client.Tasks.GetTask(taskId);

                while (!taskResponse.Completed)
                {
                    Thread.Sleep(TimeSpan.FromSeconds(20));
                    taskResponse = Client.Tasks.GetTask(taskId);
                }

                var completedReindexResponse = taskResponse.GetResponse<ReindexOnServerResponse>();
                if (!completedReindexResponse.IsValid)
                {
                    throw new Exception($"Error reindexing: {completedReindexResponse.DebugInformation}");
                }

completedReindexResponse is coming back with isValid = false and the error I mentioned above in the debugInformation

Invalid NEST response built from a null ApiCall which is highly exceptional, please open a bug if you see this

What am I doing wrong?

2

Full code for my method

    public override async Task ReindexAsync()
    {

        var oldId = this.UniqueId;
        this.UniqueId = Guid.NewGuid().ToString("N");
        var result = await Client.Indices.RefreshAsync(oldId);
        var createIdxResult = await Client.Indices.CreateAsync(
            this.UniqueId,
            i =>
                i
                    .Settings(s => CommonElasticIndexDescriptor(s, this.Alpha2LanguageCode))
                    .Map<SearchableCmsPageItem>(m => m
                    .AutoMap()

                )
            );

        if (result.IsValid && createIdxResult.IsValid)
        {
            var reindexResponse = Client.ReindexOnServer(r => r
            .Source(s => s
                .Index(oldId)
            )
            .Destination(d => d
                .Index(this.UniqueId)
            )
            .WaitForCompletion(false)
);

            var taskId = reindexResponse.Task;
            var taskResponse = Client.Tasks.GetTask(taskId);

            while (!taskResponse.Completed)
            {
                Thread.Sleep(TimeSpan.FromSeconds(20));
                taskResponse = Client.Tasks.GetTask(taskId);
            }

            var completedReindexResponse = taskResponse.GetResponse<ReindexOnServerResponse>();
            if (!completedReindexResponse.IsValid)
            {
                throw new Exception($"Error reindexing: {completedReindexResponse.DebugInformation}");
            }
            var realiasResult = await Client.Indices.BulkAliasAsync(b => b
                   .Remove(r => r
                       .Alias(this.Alias)
                       .Index(oldId))
                   .Add(a => a
                       .Alias(this.Alias)
                       .Index(this.UniqueId)));
            if (!realiasResult.IsValid)
            {
                throw new Exception($"Error swapping index alias from {oldId} to {this.UniqueId} indexes");
                // what to do here? Delete the new index and pass retry logic up or retry here?
            }
            else
            {
                try
                {
                    var delRes = Client.Indices.Delete(oldId);
                    if (!delRes.IsValid)
                    {
                        throw new Exception($"Failed to delete Index {oldId}");
                    }
                }
                catch (Exception e)
                {
                    throw e;
                }
            }

        }
    }
3

The IsValid response of an IResponse deserialized with .GetResponse<T>() on a GetTaskResponse (that's a bit of a mouthful :smiley:) cannot be used or relied upon because it uses parts of the HTTP response in determining validity, such as HTTP status code, and the HTTP response details available relate to the GetTaskResponse.

The XML documentation comment on GetResponse<T> mentions this

4

Fantastic! Thanks, that makes sense!

5

No worries!

6

OK, that works like a charm now i understand it, but it's not intuitive. I understand sometimes design decisions need to be made that aren't optimal, but combined with sparse documentation on the nest client this will throw noobs like me who are wanting to do something basic like reindex in .Net and will drive people from using the client. Please accept this as constructive criticism, I'm really keen to drive adoption of elastic search in our platform (not just for search) and an continuing to champion this. I certainly appreciate the quick and constructive help I've gotten from the elastic team while I've been exploring your nest client and elastic in general! :slight_smile:

7

Thank you for raising, I genuinely appreciate the feedback! :elasticheart:

I'd like to explain how the design has come about, as I think it'd shed some light on why it currently is as it is. I agree that it could be more intuitive in the future.

The GetResponse<T>() method was introduced in 7.2.0 in PR #3966. Since it's possible that T could be any type of response that currently exists, or might exist in the future for a new task based API, it was decided to constrain T to types that implement IResponse, the common interface for responses.

A downside in taking this approach is that IResponse contains some members that won't be valid to be used in the GetResponse<T>() context, specifically, those that derive their values from properties of the underlying HTTP response, such as status codes, headers, etc. such as IsValid, whose default implementation checks HTTP status codes

Another approach to tackling this would be to split the IResponse (and IElasticsearchResponse interface that it implements) to separate out members that derive their values from properties of the underlying response from those that don't, such that we have an interface that can be used to represent a response for GetResponse<T>() and one to use for representing an overall response from Elasticsearch. This might be an approach to consider for the next major version, but we wouldn't be able to make this change within 7.2.0 or the 7.x lineage because it would introduce binary compatibility breaking changes, something that we really want to avoid within a major version because of the impact it has on packages that take a dependency on the client packages.

8

Hi Russ, thanks for the outline, can totally appreciate the decisions made. :slight_smile: :smile:

9

This topic was automatically closed 28 days after the last reply. New replies are no longer allowed.