Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map _index in Document and update/delete on the actual index for documents retrieved from an alias #3007

Open
mbrozzo opened this issue Nov 15, 2024 · 8 comments
Labels
type: enhancement A general enhancement

Comments

@mbrozzo
Copy link

mbrozzo commented Nov 15, 2024

First of all, apologies if this was already asked. I have tried to fiend other issues detailing this problem, but I have only found #2112, which seems similar but not the same.

We have a big index which is only going to grow, so we want to reindex it to an alias which points to a set of indices with automatic rollover.
We would like to only interact with the indices using the alias, but we need to update and delete specific documents in any of the indices, which Elasticsearch does not allow through the alias due to possible _id conflicts. This means that in our application we will have to manually retrieve the _index from each SearchHit and then call ElasticsearchRestTemplate.update or ElasticsearchRestTemplate.delete with the appropriate index coordinates.

I was wondering if it would be possible to:

  • use an annotation to map the value of _index inside the Document in a similar way to the @Id annotation (this means we would not need to get it manually from the SearchHits);
  • perform update, delete and save operations on the index mapped in the Document, if present (otherwise, the index used would be the indexName defined in the @Document annotation).
    The second point would also apply to repositories.

We are evaluating the possibility of extending ElasticsearchRestTemplate to override methods and include the functionality described above.

Please let me know if this sounds as a reasonable suggestion!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 15, 2024
@sothawo
Copy link
Collaborator

sothawo commented Nov 28, 2024

had no time yet to look into this

@sothawo
Copy link
Collaborator

sothawo commented Dec 1, 2024

For the first question: There already is the @IndexedIndexName from #2112 which you referred to. This maps the name of an index where a document was found to a property of the returned entity.

As for the second question to use this when writing to Elasticsearch, I still have to think this over. We cannot just add this, as this would be a breaking change for people already using this annotation (data mght getwritten some place it wasn't meant to).

@mbrozzo
Copy link
Author

mbrozzo commented Dec 5, 2024

Thank you for your response @sothawo.

According to the docs, when a property is annotated with @IndexedIndexName, said property "will not be read from an Elasticsearch document" and will only be set "After an entity is persisted" ("the entity returned from that call will contain the name of the index that an entity was saved to in that property"). If the docs are correct and I am interpreting them correctly, @IndexedIndexName would not be suitable to retrieve the name of the index where a document was found after a search.
I would test this, but at the moment it would be quite troublesome because we are still using a very old version of Spring Data Elasticsearch which does not have said property (we are in the process of updating both Elasticsearch and the library).

In my proposal, I assumed (though I failed to make it explicit) to use a new annotation (maybe just @IndexName?) in order to accomodate those features. Modifying @IndexedIndexName while keeping backwards compatibility could be troublesome, unless a new optional parameter was added to enable the new features. Something like @IndexedIndexName(useToPersist="true") to use the property value as destination index, defaulting to "false".

@sothawo
Copy link
Collaborator

sothawo commented Dec 5, 2024

You're right, I though it is set as well after a search. Changing the code to add the value which is alrfeady available in the SearchHit would be possible, because it would add some additionla information, as up to now no one would have used this property after a search.

Still not sure if using this when saving is a good idea, have to do some research about possible problems. To add it by adding an additional parameter was my thought as well, so it would not break existing behaviour on writing.

@sothawo
Copy link
Collaborator

sothawo commented Dec 9, 2024

The documentation needs an adjustment. I just found that a property annotated with @IndexedIndexName is quite well set to the value of the index after a search operation. This came as a side effect of #2756, probably not intended at that time, but well - it's working and no one ever complained since last November.

That leaves the writing part. I'll take care of that, but it might take some time (day-time job, hobbies, family and things like that)

@mbrozzo
Copy link
Author

mbrozzo commented Dec 10, 2024

Uh nice! I guess you could call that an unintended feature :v

About the writing part, with the annotation working as it does now, I think it can be achieved easily by overriding the repositoryElasticsearchOperations save and delete - edit: and update - methods with wrappers that take the index name from the annotated property and use it for the save/delete - edit: or update - operation. Therefore, I think adding the functionality to the annotation is not such an urgent matter.

Edit: while overriding methods in ElasticsearchRepository is a possible solution, ElasticsearchRepository does not provide methods that also accept an IndexCoordinates parameter, unlike ElasticsearchOperations which is probably the better choice in this case. Therefore, if one needs to use the repositories, it would definitely be useful to be able to set the destination index in the annotated property.

Edit: grammar and formatting.

sothawo added a commit to sothawo/spring-data-elasticsearch that referenced this issue Dec 11, 2024
@sothawo
Copy link
Collaborator

sothawo commented Dec 12, 2024

You'd need to write a repository fragment that implements these save methods (fragment implementations are preferred over the standard implementation) and in this fragment you could inject a ElasticsearchOperations and do the calls yourself.

I have on a local branch added the tests and adapted the documentation, thought first about to just merge that, but I will keep it until I have the saving implemented as well.

@mbrozzo
Copy link
Author

mbrozzo commented Dec 12, 2024

Thank you very much for pointing me in the right direction!

@sothawo sothawo added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 14, 2024
sothawo added a commit to sothawo/spring-data-elasticsearch that referenced this issue Jan 12, 2025
sothawo added a commit to sothawo/spring-data-elasticsearch that referenced this issue Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants