Conversation
|
@brandur actually what's your thought on whether this should just be part of |
So if the user specifies their own set of indexes to reindex and then uses Pro, Pro would append one list to the other? I guess this should be fine. The only the downside I can imagine is if you people end up coming to rely on it too much, they may request additional customization like number of concurrent reindexes, etc., and before you know it you could a more expansive set of options needed (we already have two on there just for the reindexer and hopefully it wouldn't expand anymore than that). I suppose that's not the end of the world though. It'd be sort of nice to have a way of marking a property as "contemplative, but not totally stable" in Go to give us a little leeway to change this sort of thing, but I don't think it's possible. |
bgentry
force-pushed
the
bg/reindexer-pro-extension
branch
from
62d45d4 to
b5767f9
Compare
bgentry
changed the title
bgentry
force-pushed
the
bg/reindexer-pro-extension
branch
from
b5767f9 to
c67ca19
Compare
|
@brandur I rewrote this to add a The main downside in this new implementation is the extra API to expose the default list and the potential for drift if you don't reference it and instead try to copy-paste the list's values. Speaking of which, those values are tucked into an internal package and not immediately available for click-through. I doubt many users will customize this to begin with, so it's probably not much of an issue. Thoughts? |
Yeah, seems okay I think. All the tools are there to get it right, and in case someone doesn't on their first try, it's a pretty quick fix so I wouldn't worry about it too much. |
reindexer.go
Outdated
| import "github.com/riverqueue/river/internal/maintenance" | ||
|
|
||
| // ReindexerIndexNamesDefault returns the default set of indexes reindexed by River. | ||
| func ReindexerIndexNamesDefault() []string { |
There was a problem hiding this comment.
What do you think about merging this into one of the more general files like client.go? The trouble with adding this is that now when you quick find on "reindexer" you'll have two choices in there, and this one is going to be a bit obnoxious since there's so little in it -- you'll almost always be looking for the other file.
There was a problem hiding this comment.
Yep, agreed, relocated it. I also moved the default list itself into the package so it's much easier to jump to the definition from the exported API.
|
|
||
| // ReindexerIndexNames customizes which indexes River periodically reindexes. | ||
| // If nil, River uses [ReindexerIndexNamesDefault]. If non-nil, the provided | ||
| // slice is used as the exact list. |
Some Pro features add high churn tables that would benefit from the
reindexer. As of now, however, there's no way to extend it to run on
anything but the default index list.
Add `Config.ReindexerIndexNames` so callers can provide the exact list
to reindex, and export `ReindexerIndexNamesDefault()` so integrations
can start from the built-in targets without reaching into internal
maintenance code.
Thread the configured names into `maintenance.Reindexer` and filter
them through `IndexesExist` before starting work. That lets
mixed-version or partially migrated installs skip absent indexes
instead of trying to rebuild objects that are not there. Preserve
the `nil` versus non-`nil` contract in `WithDefaults` by copying the
slice without collapsing an explicit empty override back to `nil`, so
`[]string{}` still means "reindex nothing".
When `IndexesExist` fails during reindex discovery, advance the next
scheduled run before resetting the timer. The old code reset against
the already-fired `nextRunAt`, which made `time.Until(nextRunAt)`
zero or negative and caused immediate retries in a tight error loop.
Scheduling from the prior run time preserves the configured cadence
after transient discovery failures. The added tests cover the exported
default list, exact override propagation, explicit empty overrides,
missing-index filtering, and the discovery-error retry path.
bgentry
force-pushed
the
bg/reindexer-pro-extension
branch
from
c67ca19 to
704050a
Compare
2 participants
The reindexer currently operates on a fixed list of indexes, which makes it hard to extend in deployments that have additional high-churn indexes worth maintaining.
Add
Config.ReindexerIndexNamesso callers can provide the exact reindex target list, and exportReindexerIndexNamesDefault()so integrations can compose from River's built-in defaults without reaching into internal maintenance code. Preserve the public contract thatnilmeans "use the defaults" while a non-nilslice is the exact override, including[]string{}meaning "reindex nothing".The reindexer now filters configured targets through
IndexesExist, warns when configured targets are missing, and advances the schedule after discovery errors instead of retrying immediately in a tight error loop. Tests cover the exported default list, override propagation, explicit empty overrides, missing-index filtering, and the discovery-error retry path.