ankane / searchkick

Intelligent search made easy
MIT License
6.54k stars 757 forks source link

Full Reindexes don't always clean old indices. #1688

Closed maria-lau closed 2 months ago

maria-lau commented 2 months ago

Describe the bug When calling a full async reindex where wait is set to true (e.g. SomeModel.reindex(mode: :async, wait: true)), old indices are automatically cleaned up by this line: https://github.com/ankane/searchkick/blob/614e5da0989d0df47043730720d42bd3cf5478e0/lib/searchkick/index.rb#L414

Reading the code, it looks like other full non-async reindexing (where there's pre-existing indices) also get a call to clean_indices here: https://github.com/ankane/searchkick/blob/614e5da0989d0df47043730720d42bd3cf5478e0/lib/searchkick/index.rb#L388

Whereas in the case of a full async reindex, where we don't set wait(e.g. SomeModel.reindex(mode: :async)), cleaning up old indices is not automatically done. This seems like a bug to me since we clean the indices in most other full reindexing cases where there are indices to clean up and only need to manually call it in this case?

It seems like an easy fix to me, we just need to move this line: https://github.com/ankane/searchkick/blob/614e5da0989d0df47043730720d42bd3cf5478e0/lib/searchkick/index.rb#L414 outside of the if wait block, but have it still stay within the if mode == :async block

To reproduce

Additional context Using gem version 5.2.4, but looking at the source code, it doesn't seem different with the latest.

maria-lau commented 2 months ago

I also see the following in the promote definition: https://github.com/ankane/searchkick/blob/v5.2.4/lib/searchkick/index.rb#L93-L94

which seems like it should also clean up the indices, but calling promote doesn't clean-up the indices for me.

ankane commented 2 months ago

Hi @maria-lau, moving clean_indices outside of the wait block will delete the new index (and all other non-active indices will have already been cleaned up earlier in the method). You can manually call clean_indices after promote if needed to clean up the previous index.