Skip to content

Skip write warmup in read-only mode#239

Merged
em3s merged 7 commits intomainfrom
fix/read-only-allow-sys-init
Apr 10, 2026
Merged

Skip write warmup in read-only mode#239
em3s merged 7 commits intomainfrom
fix/read-only-allow-sys-init

Conversation

@eazyhozy
Copy link
Copy Markdown
Member

@eazyhozy eazyhozy commented Apr 3, 2026

Summary

Fix read-only mode warmup: skip write operations at both HTTP and engine levels instead of blocking them with the filter.

Problem

When running with actionbase.read-only=true and warmup enabled, two write paths caused issues:

  1. HTTP warmup (ServiceLabelEdgeControllerWarmUp): Sends POST/PUT/DELETE to /graph/v2/service/sys/label/info/edge on localhost — blocked by ReadOnlyRequestFilter
  2. Engine warmup (Graph.warmUp): Performs fake edge writes to warm HBase connections — unnecessary write load on production HBase from read-only servers

Changes

  • ServiceLabelEdgeControllerWarmUp: Use GET-only warmup methods when readOnly=true
  • GraphConfig: Add readOnly: Boolean field
  • GraphConfiguration: Pass serverProperties.readOnly to GraphConfig
  • Graph.kt: Extend existing label.entity.readOnly check with || readOnly to skip engine-level write warmup
  • StartUpTest: Add E2E test for read-only=true + warmup.enabled=true combination

How to Test

./gradlew :server:test --tests "com.kakao.actionbase.server.StartUp*"
./gradlew :server:test --tests "com.kakao.actionbase.server.filter.ReadOnlyRequestFilterTest"

@eazyhozy eazyhozy self-assigned this Apr 3, 2026
@eazyhozy eazyhozy requested a review from em3s as a code owner April 3, 2026 08:29
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Apr 3, 2026
ReadOnlyRequestFilter blocked write requests to
/graph/v2/service/sys/label/info/edge which is called internally
during app startup (warmup) with POST/PUT/GET/DELETE methods for
system metadata initialization. This caused the app to fail
readiness probe in read-only mode (shadow tenants).

Allow the exact system init path while keeping all other write
operations blocked.

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@eazyhozy eazyhozy force-pushed the fix/read-only-allow-sys-init branch from 7dc1421 to 23ea163 Compare April 3, 2026 08:36
@eazyhozy
Copy link
Copy Markdown
Member Author

eazyhozy commented Apr 3, 2026

@em3s Could you review this?

This fixes a startup failure when actionbase.read-only=true. The warmup process (ServiceLabelEdgeControllerWarmUp) sends POST/PUT/DELETE to /graph/v2/service/sys/label/info/edge internally, but ReadOnlyRequestFilter was blocking them — causing readiness probe to fail.

The fix adds an exact-path exception for this single warmup endpoint.

Pod logs from a read-only deployment:

INFO  c.k.a.s.filter.ReadOnlyRequestFilter : ReadOnlyRequestFilter is active. Write operations on [/graph/v2, /graph/v3] will be rejected.
WARN  c.k.a.s.filter.ReadOnlyRequestFilter : Blocked write request in read-only mode: PUT /graph/v2/service/sys/label/info/edge
ErrorCallbackNotImplemented: WebClientResponseException$Forbidden: 403 Forbidden from PUT http://localhost:8080/graph/v2/service/sys/label/info/edge

Warmup (ServiceLabelEdgeControllerWarmUp) sends POST/PUT/DELETE
to /graph/v2/service/sys/label/info/edge during startup. This test
verifies the server boots successfully when both read-only mode and
warmup are enabled — the scenario that was failing in production.

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@em3s
Copy link
Copy Markdown
Contributor

em3s commented Apr 3, 2026

Good workaround, but can we solve it differently? For example, enabling ReadOnlyRequestFilter only after the warmup?

@eazyhozy
Copy link
Copy Markdown
Member Author

eazyhozy commented Apr 4, 2026

@em3s Thanks for the suggestion — agreed, activating the filter after warmup is a cleaner approach. I'll update the PR.

… exception

Instead of allowlisting the system init path, the filter now starts
inactive and is activated after warmup completes:

- ReadOnlyRequestFilter: starts inactive, activate() enables blocking
- ServiceLabelEdgeControllerWarmUp: calls activate() after readiness UP
- WebFilterConfig: activates on ApplicationReadyEvent when warmup is disabled
- Tests updated: verify inactive/activated behavior, remove path exception

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Apr 4, 2026
@eazyhozy
Copy link
Copy Markdown
Member Author

eazyhozy commented Apr 4, 2026

Updated — the filter now starts inactive and is activated after warmup completes, instead of allowlisting a specific path.

Changes:

  • ReadOnlyRequestFilter: starts inactive (AtomicBoolean), activate() enables blocking
  • ServiceLabelEdgeControllerWarmUp: calls activate() after readiness UP
  • WebFilterConfig: fallback ApplicationReadyEvent listener activates the filter when warmup is disabled
  • Tests: added inactive/activated unit tests + E2E test for read-only + warmup combination

@eazyhozy
Copy link
Copy Markdown
Member Author

eazyhozy commented Apr 4, 2026

Activation flow & safety:

Scenario Filter state K8s traffic Risk
Warmup success → readiness UP → activate() Active (blocking) Routed to pod None
Warmup failure → readiness DOWN Inactive (pass-through) Not routed (probe fails) None — no external requests reach the filter

activate() is a single AtomicBoolean.compareAndSet — it cannot fail or throw. If readiness UP fails, activate() is never reached, but K8s won't route traffic to the pod anyway.

- Reorder: default → mutation mode → read-only off/on → warmup combinations
- Add StartUpWithReadOnlyAndWarmUpDisabledTest (warmup=false + read-only=true)
- Rename StartUpWithReadOnlyAndWarmUpTest → StartUpWithReadOnlyAndWarmUpEnabledTest

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Apr 4, 2026
@eazyhozy
Copy link
Copy Markdown
Member Author

eazyhozy commented Apr 6, 2026

@em3s Gentle reminder — updated per your feedback (filter activates after warmup instead of path allowlist).

@em3s
Copy link
Copy Markdown
Contributor

em3s commented Apr 6, 2026

How about just adding some condition to avoid writing? @eazyhozy .

@eazyhozy
Copy link
Copy Markdown
Member Author

eazyhozy commented Apr 6, 2026

@em3s Thanks for the suggestion — it led me to reconsider the approach.

Since read-only servers don't need write warmup, disabling warmup via kc.graph.warm-up.enabled=false avoids the conflict entirely with no code changes. Would this approach work? If so, I'll close this PR.

…ctivation

- ServiceLabelEdgeControllerWarmUp: use GET-only warmup when readOnly=true
- ReadOnlyRequestFilter: remove activate()/AtomicBoolean, active from startup
- WebFilterConfig: remove readOnlyFilterActivator bean
@eazyhozy
Copy link
Copy Markdown
Member Author

eazyhozy commented Apr 7, 2026

@em3s Updated the PR based on your feedback.

Problem: When running with read-only: true, the HTTP-level write warmup (ServiceLabelEdgeControllerWarmUp) is blocked by ReadOnlyRequestFilter, causing startup errors.

Minimal fix in this PR:

  • ServiceLabelEdgeControllerWarmUp: In read-only mode, warmup uses only GET requests (skips POST/PUT/DELETE)
  • ReadOnlyRequestFilter: Removed activate() — the filter is active from startup since write warmup is no longer sent in read-only mode
  • WebFilterConfig: Removed readOnlyFilterActivator bean

Regarding Graph.kt (engine layer):
Even in server read-only mode, the engine-level warmup (Graph.warmUp) will still perform fake edge writes to warm up HBase connections. This doesn't cause startup issues (it doesn't go through the HTTP filter), but to skip it we'd need to pass the readOnly property down to GraphConfig, which expands the scope of this PR. Should we handle this here or in a separate PR?

…warmup split)

Simpler fix incoming — skip write warmup via readOnly condition instead of restructuring warmup flow.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. size:M This PR changes 30-99 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. size:S This PR changes 10-29 lines, ignoring generated files. labels Apr 10, 2026
- GraphConfig: add readOnly field
- Graph.kt: extend existing label.entity.readOnly check with || readOnly
- GraphConfiguration: pass serverProperties.readOnly to GraphConfig
- ServiceLabelEdgeControllerWarmUp: use GET-only methods when readOnly
- StartUpTest: add E2E test for read-only + warmup enabled combination

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@em3s em3s force-pushed the fix/read-only-allow-sys-init branch from fa3254c to f7f0c2a Compare April 10, 2026 06:28
@em3s
Copy link
Copy Markdown
Contributor

em3s commented Apr 10, 2026

@eazyhozy

Pushed a commit based on my earlier suggestion — extending the if (label.entity.readOnly) condition in Graph.kt. Please review and update the PR description accordingly.

@em3s em3s removed their request for review April 10, 2026 06:35
@eazyhozy
Copy link
Copy Markdown
Member Author

@em3s Thanks for pushing the Graph.kt change — this covers the engine-level warmup I was unsure about scoping in. LGTM, updated the PR description to reflect the final approach.

@eazyhozy eazyhozy changed the title fix: allow system metadata init path in read-only mode fix: skip write warmup in read-only mode Apr 10, 2026
@em3s em3s changed the title fix: skip write warmup in read-only mode Skip write warmup in read-only mode Apr 10, 2026
@em3s em3s merged commit f0a84e8 into main Apr 10, 2026
7 checks passed
@em3s em3s added this to the v0.3.0 milestone Apr 21, 2026
em3s pushed a commit that referenced this pull request Apr 21, 2026
Backport read-only mode to `0.2.x`

- Cherry-pick #230, #239
- Register `POST /graph/v2/query` in `ReadOnlyRequestFilterTest` (0.2.x-only)

#236 excluded: 0.2.x has no `edges/cache → edges/seek` rename.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants