Graylog2 / Graylog2/graylog2-server

LookupTableService: unserialized event handlers on shared thread pool cause stale or broken lookup tables

Open
#25,731 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Java
Stars
8.1k
Forks
1.1k
Avg merge
1d 20h
Merged PRs (30d)
217

Description

## Summary

`LookupTableService` manages in-memory lookup state across 5 `ConcurrentHashMap`s, but all 7 `@Subscribe` event handlers dispatch work to the shared `@Named("daemonScheduler")` — a 30-thread `ScheduledThreadPoolExecutor`. Concurrent events (adapter update + cache update, bulk import, content pack install) execute overlapping handlers on different threads with no serialization, leading to permanently broken lookup tables that reference stopped services.

## Root Cause

Each event handler (`handleAdapterUpdate`, `handleCacheUpdate`, `handleLookupTableUpdate`, etc.) follows this pattern:

```java
@Subscribe
public void handleAdapterUpdate(DataAdaptersUpdated updated) {
scheduler.schedule(() -> {
// 1. Create new adapter, start it, wait for RUNNING
// 2. Put into idToAdapter + liveAdapters (two separate ConcurrentHashMap puts — not atomic)
// 3. createLookupTable() for all tables using this adapter
// - reads from idToCache (may see old or new cache depending on timing)
// - reads from idToAdapter
// - liveTables.put() — last writer wins
// 4. Stop old adapter
}, 0, TimeUnit.SECONDS);
}
```

The `daemonScheduler` has a pool size of 30 (`SchedulerBindings.java:36`), shared across the entire server. When multiple lookup-related events arrive concurrently, their handlers run on different threads and modify overlapping state without coordination. The 5 `ConcurrentHashMap`s provide per-operation atomicity but no compound-operation guarantees across maps.

### Why ConcurrentHashMap is insufficient

The handlers perform multi-step sequences that must be atomic:

1. **Cross-map consistency:** `idToAdapter.put()` and `liveAdapters.put()` happen in separate calls (`DataAdapterListener.running()`, lines 203-204). Between them, a concurrent `createLookupTable()` can see the new adapter in one map and the old in the other.

2. **Read-then-build-then-write in `createLookupTable()`:** Reads `idToCache.get()` (line 527) and `idToAdapter.get()` (line 533), builds a `LookupTable`, then writes to `liveTables.put()` (line 571). A concurrent delete or update can invalidate the read values before the write completes.

3. **Last writer wins across handlers:** When both an adapter update and a cache update affect the same lookup table, both handlers call `createLookupTable()`. Each builds a `LookupTable` from whatever it sees in the maps at that moment. The last `liveTables.put()` wins — potentially installing a table built from a stale snapshot.

4. **Old service stopped while still referenced:** After replacing the table in `liveTables`, the handler calls `stopAsync()` on the old adapter/cache. But message processing threads that already obtained a reference to the old `LookupTable` are still using the old (now stopping) service, causing `IllegalStateException` from `checkState(isRunning())` in `LookupDataAdapter.get()` (line 141).

## Customer Impact

**Severity: High — lookup tables can become permanently broken**

### Hard failures on the message processing hot path

When a `LookupTable` ends up referencing a stopped adapter, every pipeline rule calling `lookup()` on that table throws `IllegalStateException`. This causes:
- Pipeline rule evaluation failures logged as errors
- Messages processed without enrichment data (missing GeoIP, threat intel, asset lookups)
- Depending on pipeline error handling configuration, messages may be dropped entirely

### Permanently broken state

Unlike transient errors, this state does **not self-heal**. The `liveTables` map holds a reference to a stopped adapter. The correct adapter is running and registered in `idToAdapter`/`liveAdapters`, but never wired into the lookup table. Recovery requires:
- Manually re-saving the affected adapter or cache config (triggers a new event)
- Restarting the Graylog node (full rebuild from DB)

A customer may not realize the state is broken, since the UI reads from MongoDB (which has the correct config) while the runtime uses the stale in-memory state.

### Non-deterministic across cluster nodes

On a multi-node cluster, the race depends on per-node thread scheduling. Different nodes can end up in different states — some working, some broken — for the same lookup table. This makes the issue extremely difficult to diagnose: lookups succeed on some nodes and fail on others, with the outcome depending on which node processes each message.

### Triggered by common operations

The race window opens during any concurrent lookup-related changes:
- **Content pack / Illuminate pack installation** — creates multiple adapters, caches, and tables in rapid succession, firing 10+ events
- **Admin updating adapter + cache for the same table** — two clicks in the UI
- **API automation** — scripts that configure multiple lookup entities
- **Bulk operations** — any operation that touches multiple lookup entities

### Cascade impact through pipelines

A broken lookup table affects every pipeline rule that references it. In environments where lookup tables provide enrichment (threat intel, GeoIP, asset mapping), a single broken table can degrade enrichment for all messages passing through that pipeline stage.

## Suggested Fix

### Recommended: Dedicated single-threaded executor with coalescing

Replace `scheduler.schedule(() -> {...}, 0, TimeUnit.SECONDS)` in all 7 event handlers with submission to a **private single-threaded executor** dedicated to lookup state mutations.

```java
private final ExecutorService lookupStateExecutor = Executors.newSingleThreadExecutor(
new ThreadFactoryBuilder().setNameFormat("lookup-state-updater-%d").setDaemon(true).build());
```

This serializes all state mutations, eliminating every compound-operation race without any need for fine-grained locking across the 5 maps.

**Additionally**, add coalescing to batch rapid events. When 10 events arrive in 50ms (content pack install), they should trigger a single rebuild pass rather than 10 overlapping ones. A simple approach: accumulate event IDs for a short window (e.g., 100ms), then process the batch.

### For the old-adapter-still-referenced issue (Issue 4 above)

Delay `stopAsync()` on old adapters/caches. Instead of stopping immediately after replacement, schedule the stop with a short delay (e.g., 5 seconds) to allow in-flight lookups to complete. Or use reference counting to stop only when no thread holds the old reference.

## Effort Estimate

- **Single-threaded executor (core fix):** Small — ~2-4 hours. Replace the scheduler dispatch in all 7 handlers with the dedicated executor. This is the same pattern needed in several other places in the codebase that use the shared `daemonScheduler` for state mutations.
- **Coalescing:** Small — ~2-4 hours. Add a short batching window to reduce redundant rebuilds during bulk operations. This is a performance improvement on top of the core fix.
- **Delayed stop for old services:** Small — ~1-2 hours. Add a scheduled delay before calling `stopAsync()` on replaced adapters/caches.
- **Testing:** Medium — ~4-8 hours. Requires a concurrent stress test: multiple threads firing adapter/cache/table update events while lookup threads perform continuous lookups. Verify that `liveTables` always reflects the latest state and that no `IllegalStateException` is thrown.

**Total estimate: 1-2 days** including testing.

## Affected Code

- `LookupTableService.java` — all 7 `@Subscribe` handlers (lines 300-411), `DataAdapterListener.running()` (lines 200-212), `CacheListener.running()` (lines 260-272), `createLookupTable()` (lines 526-576)
- `SchedulerBindings.java` — `SCHEDULED_THREADS_POOL_SIZE = 30` (line 36) — the shared pool that enables concurrent execution
- `LookupDataAdapter.java` — `get()` (line 137-145) — `checkState(isRunning())` that throws when a stopped adapter is called

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.