fix(search): persist FT.SYNUPDATE synonyms across server restarts #5988
Conversation
vyavdoshenko
requested review from
BorysTheDev,
dranikpg,
mkaruza and
romange
src/server/rdb_load.cc
Outdated
| } | ||
|
|
||
| // Prepend FT.CREATE to index definiton | ||
| // Determine command type and prepend appropriate prefix |
There was a problem hiding this comment.
what is the aux format for these definitions?
will it stay compatible with previous snapshots?
will it be extendable with more commands?
There was a problem hiding this comment.
Fixed.
Aux format: Two separate fields - "search-index" for FT.CREATE, "search-synonyms" for FT.SYNUPDATE
Backward compatible: Yes. Tested with the main branch.
Extensible: Yes. Can add search-aliases, search-config, etc., as separate aux fields using the same pattern.
src/server/rdb_save.cc
Outdated
| // Convert set<string> to vector for joining | ||
| std::vector<std::string_view> terms_vec(terms.begin(), terms.end()); |
There was a problem hiding this comment.
I'm almost sure that StrJoin works with as set as well?
src/server/rdb_load.cc
Outdated
| string command_name; | ||
| if (!arg_vec.empty()) { | ||
| string_view first_token = facade::ToSV(arg_vec[0]); | ||
| if (first_token == "SYNUPDATE") { |
There was a problem hiding this comment.
Why not just pass a flag to LoadSearchIndexDefFromAux instead of all the ifs?
There was a problem hiding this comment.
Added a separate method.
src/server/rdb_save.cc
Outdated
| // Format: "SYNUPDATE index_name group_id term1 term2 term3" | ||
| std::string syn_cmd = absl::StrCat("SYNUPDATE ", index_name, " ", group_id, " ", | ||
| absl::StrJoin(terms_vec, " ")); |
There was a problem hiding this comment.
we also don't need the SYNUPDATE prefix if we have a special aux header for it
Synonyms created with
FT.SYNUPDATEwere lost after the Dragonfly restart, causing data loss.Solution:
SYNUPDATEcommands alongside index definitionsFT.SYNUPDATEcommands during RDB restore