Skip to content

feat(elasticsearch): engine-managed plugins + config mount + memory cap#100

Merged
ikeikeikeike merged 6 commits into
mainfrom
feat/es-plugin-config-mechanism
Jul 12, 2026
Merged

feat(elasticsearch): engine-managed plugins + config mount + memory cap#100
ikeikeikeike merged 6 commits into
mainfrom
feat/es-plugin-config-mechanism

Conversation

@ikeikeikeike

Copy link
Copy Markdown
Member

What

Lets an engine declare plugins it needs before start, and hardens the
elasticsearch docker backend against host-wide OOM. All three additions
target the same failure mode a heavily-parallel worktree setup hits — an
ES container that either falls over (OOM) or comes up without the
analyzer plugin an index mapping requires.

  • engines[].plugins: [{id, location}] — typed, wire-level
    (repeated PluginSpec plugins = 7, mirroring initial_resources).
    The elasticsearch backend generates Elasticsearch's own official
    elasticsearch-plugins.yml and bind-mounts it in; ES installs them
    idempotently on boot (no hand-rolled entrypoint). mysql/redis/postgres/compose ignore it.
  • extras.es.config_mount (+ es.config_mount_target) — bind-mounts
    a host dir of auxiliary plugin data (e.g. an analyzer dictionary that
    plugins.yml does not cover), read-only. Resolves against the raw
    worktree root, same as kind: compose's compose.file.
  • extras.es.mem_limit — Docker --memory cap, default 2x heap
    (Elastic guidance: heap ≤ 50% of container memory).

Design note for review

ProtocolVersion 2 → 3 for the additive plugins field. The field is
proto3-additive (backward compatible), but CONTRACT.md's rule is "every
post-v0.4.0 field rides along with a bump", and the host + all
bough-plugin-* binaries co-ship from one release, so the bump only
turns an accidental partial upgrade into a loud handshake error. Flagging
in case you'd rather scope bumps strictly to incompatible changes.

Verification

  • Unit: plugins.yml rendering (official/third-party/empty-noop),
    config_mount resolution (relative/absolute/custom-target/error),
    mem-limit derivation. go test ./... -race green.
  • Real docker E2E (-tags integration): a declared plugin actually
    installs (verified via _cat/plugins), config_mount lands with real
    content, Down removes the container. PASS.
  • ES conformance (real docker + go-plugin handshake at ProtocolVersion 3):
    full lifecycle + all faults. PASS (30.96s).
  • gofumpt clean on touched files, golangci-lint run ./... 0 issues.

v0.11.0 candidate.

Adds a typed, engine-optional plugin list to the Up wire contract,
mirroring initial_resources' shape (repeated PluginSpec plugins = 7).
Each entry is an {id, location} pair; a plugin that does not manage
plugins ignores the list.

Bumps EngineProvider ProtocolVersion 2 -> 3 for the additive field, per
CONTRACT.md's "every post-v0.4.0 field rides along with a bump" rule.
bough co-ships the host and all bough-plugin-* binaries from one
release, so the bump only turns an accidental partial upgrade (a stale
plugin binary on PATH) into a loud handshake error rather than a
silently-dropped field. MagicCookieValue stays constant (it is the
plugin-type identity, not the compatibility gate).
Adds the YAML-facing EnginePlugin{id, location} type and the
Engine.Plugins field (validated per-entry via `dive`), plus
toPluginSpecs() in the create path to convert it into the wire
PluginSpec — mirroring InitialResource/toResourceSpecs exactly.
Engines that do not consume plugins are unaffected (the list is empty).
Three related hardening changes to the elasticsearch docker backend, all
addressing what a heavily-parallel worktree setup hits — an ES container
that either falls over (OOM) or comes up without the analyzer plugin an
index mapping needs:

- Consume UpReq.Plugins by generating Elasticsearch's own official
  elasticsearch-plugins.yml and bind-mounting it into the container's
  config dir (read-only). ES installs/upgrades them idempotently on
  boot, so there is no hand-rolled entrypoint or install-marker to
  maintain. Empty list => no file, no mount (pre-existing behaviour).
- extras.es.config_mount (+ optional es.config_mount_target) bind-mounts
  a host directory of auxiliary plugin data (e.g. an analyzer dictionary
  that plugins.yml does not cover) into the config dir. Resolves against
  the raw worktree root, the same convention kind: compose's
  compose.file uses.
- extras.es.mem_limit caps container memory via Docker's --memory limit,
  defaulting to 2x the JVM heap (Elastic's guidance: heap <= 50% of
  container memory). Without it one worktree's ES can grow unbounded and
  tip the whole Docker Desktop VM into its OOM killer, which then kills
  containers indiscriminately instead of the one misbehaving.

Unit tests cover plugins.yml rendering (official + third-party, empty
no-op), config_mount resolution (relative/absolute/custom-target/error
paths), and the memory-limit derivation. A real-docker integration test
(-tags integration) proves a declared plugin actually installs (via
_cat/plugins) and the config mount lands with its real content.
Document engines[].plugins, extras.es.config_mount, and extras.es.mem_limit
with a commented elasticsearch example in the README config, and the
v0.11.0 CHANGELOG entry.
Comment thread internal/config/config.go
// Plugins lists engine plugins (e.g. an Elasticsearch analyzer) to
// make available before Up. Ignored by engines that do not
// implement plugin management (mysql, redis, postgres, compose).
Plugins []EnginePlugin `yaml:"plugins" validate:"dive"`

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correctness (CONFIRMED): この plugins は docker backend しか読まない。backend 未指定時 backend.Detect は nix を優先 (detect.go:63) し、elasticsearch.go Up は extras["backend"]=="docker" の時だけ docker path に入るため、nix host では宣言した plugin が無言で drop される。この doc コメントの「elasticsearch は honor する」という含意と矛盾。→ nix Up 側で len(req.Plugins)>0 なら actionable error を返すよう修正。

Binds: binds,
PortBindings: portBindings,
Resources: container.Resources{
Memory: memLimitBytes,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correctness (CONFIRMED): 既定 2x heap の --memory cap を、従来 unbounded だった全 ES container に無条件適用。default 1g heap → 2g cap で、make setup-es/esreindex 等の bulk 作業が 2g 超で OOM-kill (exit137)RestartPolicy:no で復帰せず reindex が途中失敗しうる。→ 既定を max(2*heap, heap+1GiB) にし heap 上に最低 1GiB の headroom を確保。

if err != nil || limit <= 0 {
return 0, fmt.Errorf("invalid es.mem_limit %q from extras (want e.g. 2g, 1500m): %w", v, err)
}
return limit, nil

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correctness (CONFIRMED): es.mem_limit が heap 未満でも通す cross-check 無し。例: es.heap=3g + es.mem_limit=2g で container cap < -Xmx となり起動時 OOM、RestartPolicy:no で dead のまま ReadyCheck が timeout まで空 poll し「not ready」という誤誘導 error に。→ resolved limit < heapBytes を error 化。

if v := req.Extras["es.mem_limit"]; v != "" {
limit, err := units.RAMInBytes(v)
if err != nil || limit <= 0 {
return 0, fmt.Errorf("invalid es.mem_limit %q from extras (want e.g. 2g, 1500m): %w", v, err)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correctness (CONFIRMED): es.mem_limit="0" (や heap="0"、heapSizePattern が通す) で units.RAMInBytes(0, nil) を返し、limit<=0 枝が err==nil を %w で wrap': %!w(<nil>)' の壊れた文言 + errors.Unwrap=nil。→ nil-err ケースを %v/平文に分離。

if target == "" {
target = dockerConfigDir + "/" + filepath.Base(src)
}
return src + ":" + target + ":ro", nil

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correctness (PLAUSIBLE): config_mount を read-only bind する際、datadir と違い readability/permission を見ていない。umask 077 で用意した 0700 dir だと in-container uid 1000 が読めず analyzer 辞書 load が失敗しうる(診断メッセージ無し)。→ 今回は note に留め、必要なら後続で o+rx チェックを追加。

Comment thread plugins/engine/elasticsearch/docker.go Outdated
if req.WorktreeRoot == "" {
return "", errors.New("es.config_mount is a relative path but WorktreeRoot is empty")
}
rawWorktreeRoot := filepath.Dir(req.WorktreeRoot)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleanup (CONFIRMED): raw-worktree-root 相対解決が compose/lifecycle.go に2箇所 + この ES で計3コピー。convention 変更時に取りこぼす。→ plugins/engine/apiResolveUnderRawWorktreeRoot を切り出し3箇所で共用。

- nix backend: return an actionable error when plugins: are declared but
  the resolved backend is nix (which auto-detect prefers) instead of
  silently dropping them — a green worktree that fails at the first query
  referencing the missing analyzer.
- es.mem_limit: reject a value below the JVM heap (a cap under -Xmx
  OOM-kills the container at startup, then ReadyCheck just times out with
  a misleading "not ready").
- default memory cap: use max(2x heap, heap + 1GiB) so even a small heap
  keeps Elastic's recommended 1-2GB above-heap headroom for Lucene / the
  filesystem cache / bulk workloads (esreindex), rather than 2x-only which
  gives under 1GB once the heap drops below 1GB.
- pickMemoryLimitBytes: stop fmt.Errorf("...%w", nilErr) for a non-positive
  size (es.mem_limit/heap "0" parses to (0, nil)) — the branches now emit a
  clean message instead of ': %!w(<nil>)'.
- DRY: extract the raw-worktree-root relative-path resolution triplicated
  across compose (Up + Down) and the new ES config_mount into
  api.ResolveUnderRawWorktreeRoot; all three call it now.
- doc: note that plugins.yml (like image/env) only applies on a fresh Up,
  not into an already-running container under up-or-reuse.

Unit tests extended (mem-limit headroom floor, below-heap rejection,
zero-size clean error). Real-docker E2E + go test ./... -race stay green.
…-mechanism

# Conflicts:
#	CHANGELOG.md
#	internal/cli/create.go
@ikeikeikeike
ikeikeikeike merged commit 19c59a8 into main Jul 12, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant