docs(adr): non-breaking search index changes - #3192
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Coverage variation | ✅ +0.00% coverage variation (-1.00%) |
| Diff coverage | ✅ ∅ diff coverage |
Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (7de9ade) Report Missing Report Missing Report Missing Head commit (e15a888) 82472 (+0) 19108 (+0) 23.17% (+0.00%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>
Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#3192) 0 0 ∅ (not applicable) Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
1 Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
| fixed timeout, and a single failing space must not kill the whole run. | ||
| 5. Add the sticky message to the web UI, publishable by the operator. | ||
| 6. Document the reindex, `opencloud search index --all-spaces`, and the cleanup of old | ||
| indexes in the admin docs. |
There was a problem hiding this comment.
Do we need --rescan or is that implied when the file is not in the index at all?
There was a problem hiding this comment.
i think in the scope of the pr, no, its about "new indexes", but let me know if you see any scenario where it makes sense
There was a problem hiding this comment.
tbh from the top of my head I don't know what exactly reindexing does in detail.
My point was: just reading metadata from xattrs and writing it to the index is not enough, we need to send it to tika for fulltext content because that's the one thing that is not stored in xattrs (afaik)
If simple indexing without --rescan gets us fulltext search, we're good
77fc94a to
a7ecec6
Compare
| version, so a breaking mapping change automatically means a new, empty index. | ||
| `SEARCH_ENGINE_OPEN_SEARCH_RESOURCE_INDEX_NAME` becomes the prefix of that name | ||
| instead of the full name. A configured index can then never be the wrong version, | ||
| there is only "this version doesn't exist yet", which is the normal path. |
There was a problem hiding this comment.
| halfway. | ||
|
|
||
| Old indexes stay around and cost storage. Removing them is the operator's call as well, | ||
| we only document how. |
There was a problem hiding this comment.
To assist the operator I think we should have a simple tool (e.g. a subcommand to opencloud search) that provides info about which indexes are obsoslete and can be deleted and which is the current one.
There was a problem hiding this comment.
I like thls, but lets do it in a followup
When we implement such a status command, there's probably not much more to do to also provide a --prune switch
There was a problem hiding this comment.
I like thls, but lets do it in a followup
Fine with me. Still think we should write it down in the ADR.
There was a problem hiding this comment.
No objections obviously :)
| change that makes the index unusable gets its own version and therefore its own | ||
| index, so what still reaches this path is additive, and those fields stay empty until | ||
| the next reindex. The error stays as a strict mode for development, where failing | ||
| loudly on a mapping change without a version is what we want. |
There was a problem hiding this comment.
Part of follow up pr:
| index, so what still reaches this path is additive, and those fields stay empty until | ||
| the next reindex. The error stays as a strict mode for development, where failing | ||
| loudly on a mapping change without a version is what we want. | ||
| 3. Log that a reindex is pending when a new index is created, and name the command. |
There was a problem hiding this comment.
| loudly on a mapping change without a version is what we want. | ||
| 3. Log that a reindex is pending when a new index is created, and name the command. | ||
| 4. Make the reindex fit for a long run. It has to report progress, survive longer than a | ||
| fixed timeout, and a single failing space must not kill the whole run. |
| 5. Add the sticky message to the web UI, publishable by the operator. In progress in | ||
| https://github.com/opencloud-eu/opencloud/pull/3189. | ||
| 6. Document the reindex, `opencloud search index --all-spaces`, and the cleanup of old | ||
| indexes in the admin docs. Owned by the docs team. |
| halfway. | ||
|
|
||
| Old indexes stay around and cost storage. Removing them is the operator's call as well, | ||
| we only document how. |
There was a problem hiding this comment.
I like thls, but lets do it in a followup
When we implement such a status command, there's probably not much more to do to also provide a --prune switch
ADR for handling breaking search index changes: instead of failing on startup, the search service creates a new, empty index and leaves the old one alone, so the instance keeps running.
The reindex stays a manual step, announced to the operator via release notes and to the users via a sticky message in the web UI.
Context: #2659 (comment)