From b5626e17e647c943bdb8bffcf341c8e044fe1656 Mon Sep 17 00:00:00 2001 From: Lucas Manuel Rodriguez Date: Wed, 8 Oct 2025 06:36:38 -0300 Subject: [PATCH] Fix lingering live queries keys in Redis (#33928) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves #33254 This can be reproduced locally by running the following "high load" test: Run 500 hosts using osquery-perf: ``` go run ./cmd/osquery-perf --enroll_secret ... \ --host_count 500 \ --server_url https://localhost:8080 \ --live_query_fail_prob 0.0 \ --live_query_no_results_prob 0.0 \ --orbit_prob 0.0 \ --http_message_signature_prob 0.0 ``` Run `stress_test_live_queries.sh`: ``` #!/bin/bash while true; do curl -v -k -X POST -H "Authorization: Bearer $TEST_TOKEN" https://localhost:8080/api/latest/fleet/queries/$SAVED_QUERY_ID/run -d '{"host_ids": [<500 comma-separated host ids>]}' done ``` Use "Redis Insight" or the like and you will start to see `livequery:{$CAMPAIGN_ID}` keys with `No limit` (which is the bug): Screenshot 2025-10-07 at 3 10 26 PM - [X] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually ## Summary by CodeRabbit - Bug Fixes - Prevent lingering Redis keys for live queries by ensuring keys are cleaned up and not recreated when completing/canceling non-existent queries. - Improves resource usage and avoids stale state in live query processing. - Tests - Added tests verifying proper retrieval/completion behavior and that no Redis key is created for non-existent live queries. --------- Co-authored-by: Ian Littman --- changes/33254-prevent-lingering-redis-keys | 1 + server/live_query/live_query_test.go | 43 ++++++++++++++++++++++ server/live_query/redis_live_query.go | 13 ++++++- 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 changes/33254-prevent-lingering-redis-keys diff --git a/changes/33254-prevent-lingering-redis-keys b/changes/33254-prevent-lingering-redis-keys new file mode 100644 index 0000000000..1dac13540d --- /dev/null +++ b/changes/33254-prevent-lingering-redis-keys @@ -0,0 +1 @@ +* Fixed a bug in live queries that caused `livequery:{$CAMPAIGN_ID}` Redis keys to not be cleaned up or expire. diff --git a/server/live_query/live_query_test.go b/server/live_query/live_query_test.go index 73f64593de..aa57bcca83 100644 --- a/server/live_query/live_query_test.go +++ b/server/live_query/live_query_test.go @@ -19,6 +19,7 @@ var testFunctions = [...]func(*testing.T, fleet.LiveQueryStore){ testLiveQueryExpiredQuery, testLiveQueryOnlyExpired, testLiveQueryCleanupInactive, + testLiveQuerySetBitOnlyIfKeyExists, } func testLiveQuery(t *testing.T, store fleet.LiveQueryStore) { @@ -221,3 +222,45 @@ func testLiveQueryCleanupInactive(t *testing.T, store fleet.LiveQueryStore) { require.NoError(t, err) require.Empty(t, m) } + +func testLiveQuerySetBitOnlyIfKeyExists(t *testing.T, store fleet.LiveQueryStore) { + // Create a live query campaign. + err := store.RunQuery("test", "SELECT 1;", []uint{1}) + require.NoError(t, err) + + // Get the query for the host. + queries, err := store.QueriesForHost(1) + require.NoError(t, err) + require.Equal(t, + map[string]string{ + "test": "SELECT 1;", + }, + queries, + ) + + // Mark query as completed by host. + err = store.QueryCompletedByHost("test", 1) + require.NoError(t, err) + + // Query should not be returned anymore as it was marked as completed for this host. + queries, err = store.QueriesForHost(1) + require.NoError(t, err) + require.Empty(t, queries) + + // A host could be attempting to write a result for a query that was already deleted. + err = store.QueryCompletedByHost("test-2", 1) + require.NoError(t, err) + + // Let's test that such key was not created. + + // get a raw Redis connection to make direct checks + pool := store.(*redisLiveQuery).pool + conn := redis.ConfigureDoer(pool, pool.Get()) + t.Cleanup(func() { + conn.Close() + }) + + n, err := redigo.Int(conn.Do("EXISTS", queryKeyPrefix+"{test-2}")) + require.NoError(t, err) + require.Zero(t, n) +} diff --git a/server/live_query/redis_live_query.go b/server/live_query/redis_live_query.go index b54c1c1f3a..c5538ac9b1 100644 --- a/server/live_query/redis_live_query.go +++ b/server/live_query/redis_live_query.go @@ -263,8 +263,17 @@ func (r *redisLiveQuery) QueryCompletedByHost(name string, hostID uint) error { targetKey, _ := generateKeys(name) - // Update the bitfield for this host. - if _, err := conn.Do("SETBIT", targetKey, hostID, 0); err != nil { + // Update the bitfield for this host only if the key exists. + // If the key doesn't exist (e.g. query marked as completed or cancelled) + // then we don't want to call SETBIT because it will create a new + // key (that won't expire and linger "forever"). + const setBitScript = ` + if redis.call('EXISTS', KEYS[1]) == 1 then + return redis.call('SETBIT', KEYS[1], ARGV[1], ARGV[2]) + else + return nil + end` + if _, err := conn.Do("EVAL", setBitScript, 1, targetKey, hostID, 0); err != nil { return fmt.Errorf("setbit query key: %w", err) }