mirror of
https://github.com/mattermost/mattermost.git
synced 2026-08-27 05:37:15 -05:00
Alejandro García MontoroandMattermost Build
a2a896a5de
* Add HealthCheck to search engine interface The watcher (next commit) needs a way to probe whether ES/OS is still reachable. Each backend does a cluster health request with a 5 s timeout, snapshotting the client under the read lock so the network call doesn't block Stop()/Start(). * Add search engine background watcher The old fire-and-forget ps.Go() calls for ES startup had no retry, no health monitoring, and no backoff. If Start() failed at boot the engine stayed down until the next config change or server restart. Replace with a single watcher goroutine that: - retries Start() with exponential backoff (15 s–5 m) - runs periodic health checks once active - stops the engine after N consecutive failures - reacts immediately to config/license changes - shuts down cleanly via context cancellation * Add tests for search engine watcher Covers retry-then-health transition, exponential backoff capping, disable/enable park-unpark via notify, intermittent vs threshold health failures, and edge cases (Start ok but not active, rapid config changes, failure counter reset). * Address review comments * make i18n-extract * Make Stop() a no-op if already stopped Both Elasticsearch and OpenSearch Stop() methods returned an error when the engine was already stopped. This forced every caller to handle or ignore a non-error condition. Returning nil instead is more idiomatic and simplifies all call sites. * Simplify config listener branches Merge the connectionChanged and startingES/stoppingES branches into one. Since Stop() is now a no-op when already stopped, we can always call Stop() then notify the watcher regardless of which condition triggered the change. * Restore license listener conditional order Keep the original order (license-added first, license-removed second) to reduce diff noise. The conditions are mutually exclusive so the order has no semantic effect. * Log consecutive failures in retry phase Track and log consecutiveFailures when Start() fails or returns nil but the engine is not active, so operators can see how many attempts have been made alongside the backoff duration. * Clarify health check timer reset placement Add a comment explaining why the timer reset lives outside the if/else: both success and below-threshold failure share the same health interval, while the critical-failure path continues before reaching this point. * Remove nested select in disabled-engine path Replace the nested select block with timer.Stop() + continue. The main loop's select already handles ctx.Done() and notify, so stopping the timer is enough to park until one of those fires. * Add context.Context to Start() Accept a context in SearchEngineInterface.Start() and propagate it to all network calls (checkMaxVersion, fetchServerInfo, index template creation). This lets the watcher's cancellable context flow through to the HTTP client, so a stuck Start() call returns promptly on shutdown instead of blocking until TCP timeout. * Simplify startSearchEngineWatcher comment Focus on the actual reason (long-lived, owns its lifecycle) rather than the shutdown-blocking detail, which is less relevant now that Start() accepts a cancellable context. * Make the comment on the goroutine more accurate * Fix log * Revert the branches merge This was causing a race condition in the TestElasticsearchAggregation test. * Own Start/Stop by the engine watcher The config listener simply notifies now, so it's easier to follow the logic of the calls. * Refactor the engine watcher into its own type Simplify the code by: 1. Moving the whole watcher into its own type, so that the PlatformService contains an instance of it, instead of all the locks and channels 2. Splitting the main loop into functions. John Carmack may not like this, but it's way easier to read and follow. * make i18n-extract * Rename notify > reevaluate * Park the watcher if there is no license * Call reevaluate from within requestRestart * Use RequestTimeoutSeconds instead of hardcoded 5s * Use atomic.Int32 in the tests as well * Add defensive code ta watcher exit Stop the engine if it was still running when the watcher exits, having a safety net for scenarios like race conditions between Start and a cancelled context. --------- Co-authored-by: Mattermost Build <build@mattermost.com>