Graylog2 / Graylog2/graylog2-server

Entity sharing enumerates every content pack entity on every share

Open
#26,896 1 comment 0 reactions 0 assignees View on GitHub
backend improvement triaged
Dominant language
Java
Stars
8.1k
Forks
1.1k
Avg merge
1d 20h
Merged PRs (30d)
217

Description

## Description

Every entity share enumerates every content-pack-able entity in the installation. Bulk share paths do it once per entity, so the cost is `O(N x all entities)`.

`EntitySharesService.updatePrimaryEntityShares` calls `checkMissingPermissionsOnDependencies` unconditionally:

```java
.missingPermissionsOnDependencies(checkMissingPermissionsOnDependencies(ownedEntity, sharingUserGRN, ImmutableSet.of(), request));
```

That reaches `entityDependencyResolver.resolve(entity)` with no early exit, and `resolve` calls:

```java
private ImmutableMap> entityExcerpts() {
return contentPackEntityResolver.listAllEntityExcerpts().stream()
...
}
```

`ContentPackEntityResolver.listAllEntityExcerpts` walks every registered facade:

```java
entityFacades.values().forEach(facade -> entityIndexBuilder.addAll(facade.listEntityExcerpts()));
```

There are 18 core facades plus plugin ones, and each `listEntityExcerpts()` is a full collection read. Examples:

- `EventDefinitionFacade.listEntityExcerpts` calls `eventDefinitionService.streamAll()`
- `GrokPatternFacade.listEntityExcerpts` calls `grokPatternService.loadAll()`
- `StreamFacade.listEntityExcerpts` calls `streamService.loadAll()`, which goes through `StreamServiceImpl.loadAllByQuery` and adds a per-stream `scopeService.isMutable` check plus `streamRuleService.loadForStreamIds`, `indexSetsForStreams` and `outputService.loadByIds`

There is no cache or memoization on this path.

## Impact

Three callers loop `updateEntityShares` per entity:

| Caller | Loops over | Uses the returned `EntityShareResponse`? |
| --- | --- | --- |
| `ContentPackService.shareEntities` | every entity in the installed pack | No, the return value is discarded |
| `SigmaImportService` (enterprise) | every created event definition | No, the return value is discarded |
| `IlluminateEntitySharesService.updateShares` (enterprise) | every pack GRN | Yes |

The only consumer of the expensive work is the `missingPermissionsOnDependencies` field of `EntityShareResponse`, which the share modal uses to warn that a grantee cannot see a dependency. Two of the three bulk callers throw that response away, so on those paths the enumeration is computed N times and never read.

Illuminate is the worst case because it both installs many entities and ships thousands of grok patterns and event definitions that every subsequent enumeration re-reads.

The interactive share dialog is affected too: a single share pays one full enumeration.

## Suggested fix

Memoize `entityExcerpts()` with a short TTL. There is already a precedent for exactly this next door in `Catalog`:

```java
Suppliers.memoizeWithExpiration(getEntityExcerpts, 5, TimeUnit.SECONDS)
```

Applying the same treatment in `DefaultEntityDependencyResolver` collapses a bulk install from N enumerations to roughly one per window, requires no API change, and speeds up the interactive share dialog as well.

An alternative is to let callers that discard the response skip the dependency check, but that needs an API change and only helps two of the three callers.

## Environment

- Graylog Version: 7.2.0-SNAPSHOT (long-standing, not a regression)

Contributor guide

Open the contributing guide

Research direction

Start in DefaultEntityDependencyResolver at entityExcerpts(), then compare the existing Catalog memoizeWithExpiration(getEntityExcerpts, 5, TimeUnit.SECONDS) precedent. Trace calls from EntitySharesService.updatePrimaryEntityShares and verify that repeated bulk shares no longer enumerate every content-pack entity for each entity while the share response still reports dependency permissions.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
authorization, backend, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.