Paginate only MAX records for searchkick

[JasonBarnabe]

https://ddnexus.github.io/pagy/guides/how-to/#paginate-only-max-records describes how to have a maximum number of paginatable records for performance reasons. This works for OFFSET and KEYSET paginators.

It would be useful for me to have a way to do this with the searchkick paginator as well. ElasticSearch has an option max_result_window which defines the number of records that can be loaded by search. It defaults to 10,000 records, but searchkick provides an option to have a different value. Currently, there are two issues when using pagy in relation to max_result_window:

1. pagy offers pagination links that result in going past max_result_window.
2. When following these links that go past the window, no results show (correct) but each request still requires an ES call. (This issue may be better handled in the application and not pagy).

max_result_window is accessible from a Searchkick::Relation by search.model.searchkick_options[:max_result_window].

[ddnexus]

Ok, so I guess that the pagy :count should be the min between the total_count and the max_result_window. Is it correct?

[JasonBarnabe]

I suppose so...

Script.searchkick_options[:max_result_window]
=> 1000
Script.search('*', per_page: 2000).total_count
=> 10000
Script.search('*', per_page: 2000).count
=> 1000
Script.search('*', per_page: 2000, body_options: {track_total_hits: true}).total_count
=> 75886
Script.search('*', per_page: 2000, body_options: {track_total_hits: true}).count
=> 1000

[ddnexus]

Wait... pagy uses the total_count to set its count, while it looks like if it would use count instead, it would return the actual limited count.

If the results are just 500 I guess that count == total_count regardless the window. If that's the case the correct way to set the pagy count would be using always count.

[JasonBarnabe]

.count is the number of records on the current "page". .total_count is the total number of matches (subject to a built-in limit of 10,000 unless track_total_hits is set). max_result_window is the defined maximum record index that can be loaded.

Script.searchkick_options[:max_result_window]
=> 1000
Script.search('*', per_page: 10).count
=> 10
Script.search('*', per_page: 10).total_count
=> 10000

In this case, pagy should allow pagination up to record 1000 as the max_result_window will not allow loading anything past that.

[ddnexus]

Ok, so count is useless for pagy. The count will be set looking at the window or total_count whatever is smaller.

For ElasticSearchRails we should do the same. We have to look for the equivalent way to get the window from its options.

[ddnexus]

OK, so we need to change:

pagy/gem/lib/pagy/toolbox/paginators/searchkick.rb

Line 20 in cc37e6d

options[:count] = results.total_count

By checking model.searchkick_options[:max_result_window] which should always return an integer, right?

and:

pagy/gem/lib/pagy/toolbox/paginators/searchkick.rb

Line 28 in cc37e6d

options[:count] = search.total_count

But I have no idea whether we can get it with search.options[:max_result_window] in that block or not.

Please advise.

[JasonBarnabe]

By checking model.searchkick_options[:max_result_window] which should always return an integer, right?

If it was not set in the index definition, it could be nil, in which case I believe it's reasonable to fall back to the default 10000.

[ddnexus]

Is there a way to get it from the results instead? We need that to set the right count in passive mode anyway.

[ddnexus]

OK, I checked and there is no way to get the max_result_window from the result. It is either the default of 10_000 or whatever the user set in whatever way it did. That's not a Pagy concern then. Pagy can include a :max_result_window option defaulting to 10_000 but the user must set it to the actual value when it's not the default. Pagy will take care of it deriving the actual count.

[ddnexus]

Please, try the dev branch (see 8fc5d78)

[JasonBarnabe]

Not working as of yet. I've left some comments on that commit. Note that I am only set up to test with searchkick, not elasticsearch-rails.

[ddnexus]

@JasonBarnabe For some reason, I receive your commit comments as email, but I cannot see them in the GitHub commit page. I see the comment bubble icon in the list of files, but no comment to be seen. 🤔 (It's painful to connect the email to the actual code)

BTW, yes, the default is not properly handled, however it looks like your idea of dropping the pagy option won't work with passive mode, because Pagy has to know what was the max_result_window that generated the object, and the model is not there afaik, unless it would be accessible from the result object.

Besides, in that case, we should go after all the possible way to get the result object in Searchkick AND in ElasticsearchRails and maybe we should apply restrictions on that. Bottom line: the easiest and unequivocal way to do it is asking the user to mirror the actual max_result_sindow value if that is not 10_000.

[JasonBarnabe]

I am using it in passive mode. In total_count_from:

(byebug) response_object.class
=> Searchkick::Relation
(byebug) response_object.model.searchkick_options[:max_result_window]
=> 1000

[JasonBarnabe]

Not sure if you saw from my phantom comments, but max_result_window is per-index, not global. I'm fine with either the kind of code I show above or an argument passed in to pagy, but only having a single global setting for it doesn't work.

[ddnexus]

having a single global setting for it doesn't work

Sure. The idea is not relegating it as a global, but adding it as an option as any other, so you will be able to pass it to the constructor. Admittedly, the example in the docs could be better.

I am using it in passive mode.

Great, so it would work both ways. The only problem is that if you set it only on the elasticasearch side, it will not be reliably accessible, and you would have to mirror it manually for pagy. That's the main reason I am reluctant to try to be too smart with it. If setting it is ALWAYS a requirement when the default is different than 10_000 it will be simpler to implement and use.

[ddnexus]

I have force-pushed the fix for the wrong DEFAULT handling and the improved docs, so it will not suggest it is only a global option (see cc76ecf). Please, let me know.

[JasonBarnabe]

Looks like it works now, at least for searchkick.

[ddnexus]

Implemented in 43.6.0