From 57a8be29e3504babf7a58a33af1fb329cbf362fa Mon Sep 17 00:00:00 2001 From: Olav Groenaas Gjerde Date: Sun, 9 Aug 2026 13:33:55 +0200 Subject: [PATCH] feat: optional read replica, and one query to identify the caller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a second connection pool against a Postgres standby, off by default. `ichat.datasource.replica.enabled` gates the whole thing, and off means the beans do not exist rather than sitting idle — a deployment that never asks for one gains no proxy, no second pool and no new way to fail. Same contract as the optional Vault backend, which can now also carry the replica: db.replica-enabled, db.replica-url, db.replica-username, db.replica-password (plus db.url for the primary, which was env-only). The enabled flag is mapped deliberately, since @ConditionalOnProperty evaluates long after an EnvironmentPostProcessor runs and splitting one topology decision across Vault and the env file is how the two drift. Routing is `@Transactional(readOnly = true)` and nothing else. A LazyConnectionDataSourceProxy fronts both pools with the replica as its read-only variant; HibernateJpaDialect marks the transaction's connection read-only before the proxy has fetched a physical one, and the proxy then takes that one from the replica. The laziness is the mechanism, not a tuning choice: Hibernate checks a connection out when the transaction begins, which is before the read-only flag exists, so a pool wired straight to the EntityManagerFactory would have taken a primary connection before anything could route it. Writes, Flyway (pinned with @FlywayDataSource rather than left to follow @Primary) and raw JDBC stay on the primary. A Spring Data repository call from a non-transactional caller does not: SimpleJpaRepository declares readOnly = true, so it opens its own read-only transaction and reaches the replica. Three reads that drive deletes are moved off readOnly deliberately. The Lucene<->Postgres reconciles in CleanupTasks and LuceneBootstrap compare the two and drop what looks stale; on a lagging replica a just-committed message reads as absent from Postgres, and the sweep would delete the search document for a message that exists. Each carries a comment saying so, because the annotation looks like an oversight without one. The replica pool is read-only at the driver, not per transaction, because LazyConnectionDataSourceProxy suppresses the per-transaction setReadOnly once a dedicated read-only DataSource is configured. A startup check warns when pg_is_in_recovery() is false — the failure where "replica" points back at the primary and the deployment runs two pools against one machine believing it split the load. Second change, in the same shape: resolving a logged-in principal used to cost two queries inside a writable transaction on every authenticated request, to re-derive a row that had not changed since the request before. UserService.findUnchanged answers the same question with one select in a read-only transaction, and only when the stored row agrees with the token on every field the write path sets; anything else falls through to upsert, which is unchanged and still the only thing that writes. Both halves read claims through one shared ClaimView, because a second copy of the claim-reading logic would let them disagree about whether anything changed — a write on every request and no error anywhere. upsert also skips the collision query when the account already holds the handle, which uk_users_username_lower makes provably redundant. That fast path is what decides whether a replica is worth having. The old cost fell on every request regardless of how little it did, so it dominated exactly the light endpoints a replica is otherwise best at absorbing: a page load moves from about 80% of its reads on the replica to 90-95%, and an endpoint that only reads now puts nothing on the primary at all. It also weakens one documented invariant, so the comment claiming it no longer does. CurrentUser's suspension backstop reads a row that may be up to the replication lag old. A ban through BanService is unaffected — it updates SuspensionRegistry before writing the row, and the filter refuses the request before it reaches the backstop. What lags is what the registry cannot see: a suspended_at edited directly in psql, or a ban issued by another node. Delayed by the lag, not missed. Tests: ReadReplicaDataSourceConfigTest (7 cases) and UserProvisioningFastPathTest (10 cases) run without a container and pass. ReadReplicaRoutingIT proves the routing itself against a real Postgres — one container, two pools tagged with different ApplicationNames, asking the server which one served each transaction — and UserProvisioningIT covers the round-trip property mocks cannot reach, that upsert settles a row into exactly the shape findUnchanged tests for. Neither IT has been executed: this machine has no container runtime. Co-Authored-By: Claude Opus 5 (1M context) --- .env.example | 24 +- AGENTS.md | 6 + CHANGELOG.md | 14 +- scalability.md | 34 +++ .../chat/cleanup/CleanupTasks.java | 12 +- .../config/ReadReplicaDataSourceConfig.java | 231 ++++++++++++++++++ .../chat/config/ReadReplicaProperties.java | 71 ++++++ .../config/VaultEnvironmentPostProcessor.java | 24 +- .../chat/moderation/SuspensionRegistry.java | 4 +- .../chat/search/LuceneBootstrap.java | 7 +- .../chat/security/CurrentUser.java | 29 ++- .../chat/service/UserService.java | 102 +++++++- src/main/resources/application.yml | 30 +++ .../ReadReplicaDataSourceConfigTest.java | 145 +++++++++++ .../VaultEnvironmentPostProcessorTest.java | 34 +++ .../integration/ReadReplicaRoutingIT.java | 156 ++++++++++++ .../chat/integration/UserProvisioningIT.java | 135 ++++++++++ .../service/UserProvisioningFastPathTest.java | 182 ++++++++++++++ 18 files changed, 1204 insertions(+), 36 deletions(-) create mode 100644 src/main/java/ai/intellistream/chat/config/ReadReplicaDataSourceConfig.java create mode 100644 src/main/java/ai/intellistream/chat/config/ReadReplicaProperties.java create mode 100644 src/test/java/ai/intellistream/chat/config/ReadReplicaDataSourceConfigTest.java create mode 100644 src/test/java/ai/intellistream/chat/integration/ReadReplicaRoutingIT.java create mode 100644 src/test/java/ai/intellistream/chat/integration/UserProvisioningIT.java create mode 100644 src/test/java/ai/intellistream/chat/service/UserProvisioningFastPathTest.java diff --git a/.env.example b/.env.example index baa5572..b1f9ccc 100644 --- a/.env.example +++ b/.env.example @@ -28,6 +28,20 @@ ICHAT_DB_URL=jdbc:postgresql://localhost:5432/intellistream_chat ICHAT_DB_USERNAME=ichat_role ICHAT_DB_PASSWORD=CHANGE-ME +# Optional read replica. Off by default, and off means there is no second pool at all. Enabled, +# every @Transactional(readOnly = true) is served from the replica, while writes, Flyway and raw +# JDBC access stay on the primary above. +# +# A replica lags, so a read-only transaction may see a slightly older world than the write that +# just returned. That is fine for history and search, and it is why the Lucene reconcile sweeps +# stay on the primary. Blank username/password inherit the primary's, which is what a streaming +# replica normally wants. Size the read pool with SPRING_-style Hikari keys under +# ichat.datasource.replica.hikari.*; the shipped config sets none, so it is Boot's default of 10. +# ICHAT_DB_REPLICA_ENABLED=false +# ICHAT_DB_REPLICA_URL=jdbc:postgresql://replica.internal:5432/intellistream_chat +# ICHAT_DB_REPLICA_USERNAME= +# ICHAT_DB_REPLICA_PASSWORD= + # --- Keycloak / OIDC --- KEYCLOAK_ISSUER_URI=https://auth.your-domain/realms/ichat-realm KEYCLOAK_CLIENT_ID=ichat-client @@ -239,10 +253,12 @@ ICHAT_ASSETS_UNBUNDLED=false # Optional: Vault / OpenBao # ============================================================================================= -# Credentials only. The processor reads secret/data/${path} and maps exactly five keys — -# db.username, db.password, keycloak.client-id, keycloak.client-secret, keycloak.issuer-uri — -# into their Spring property names before anything consumes them. Any other key in the record is -# ignored on purpose, so tuning stays here in the env file. +# Connection details and credentials. The processor reads secret/data/${path} and maps exactly ten +# keys — db.url, db.username, db.password, db.replica-enabled, db.replica-url, +# db.replica-username, db.replica-password, keycloak.client-id, keycloak.client-secret, +# keycloak.issuer-uri — into their Spring property names before anything consumes them. Any other +# key in the record is ignored on purpose, so tuning stays here in the env file. Every one of them +# is optional: a record holding only db.password overrides only that. # ICHAT_VAULT_ENABLED=false # ICHAT_VAULT_URI= # ICHAT_VAULT_TOKEN= diff --git a/AGENTS.md b/AGENTS.md index e7c80cc..dd033dc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -58,6 +58,7 @@ src/main/java/ai/intellistream/chat/ │ CallService, TurnCredentialService, CallScheduler, CallSessionListener ├── config/ SecurityConfig (two filter chains), WebSocketConfig, │ StompAuthorizationConfig, MultipartConfig, VaultEnvironmentPostProcessor, +│ ReadReplicaDataSourceConfig / ReadReplicaProperties (optional replica), │ RegistrationAuthorizationRequestResolver ├── domain/ JPA entities — User, Channel, Message, Conversation, │ Attachment / ConversationAttachment, MessageReaction / ConversationReaction, @@ -108,6 +109,8 @@ Several autoconfigurations that lived inside `spring-boot-autoconfigure` in 3.x - A client subscribes once per **joined channel**, not once per rendered sidebar row, so notification coverage cannot narrow when the sidebar's rendering changes. The per-session SUBSCRIBE budget is 2000/min because over-budget frames are *dropped, not refused* — at the old 200 the tail of a large account's own subscriptions vanished with no error anywhere. - **Authorisation happens once, at SUBSCRIBE.** The broker never re-checks, so revoking access needs the subscription taken away too: `ChannelSubscriptionRevoker` (channels) and its conversation sibling do that on leave, kick, and channel delete. Evicting the access cache alone only stops the *next* subscribe. - **`CurrentUser`** is the single bridge between Spring Security principals and the domain `User`. It provisions/upserts a `User` row from the OIDC subject the first time it sees a principal. Always go through it; don't read JWT/OidcUser claims in controllers. + **It resolves in two steps, and the order matters.** `UserService.findUnchanged` is a read-only single-`select` lookup that answers only when the row already agrees with the token on *every* field the write path sets — handle, email, display name, admin. Anything else returns empty and falls through to `UserService.upsert`, which is unchanged and still the only thing that writes. This runs on every authenticated request, so the difference is the whole per-request database cost of being logged in: one replica-eligible read instead of two primary reads inside a writable transaction. Both halves read their claims through `UserService.claimsOf`, one shared `ClaimView` — a second copy of the claim-reading logic would let the two disagree about whether anything changed, which is a write on every request and no error anywhere. The fall-through is stable, not repeating: `upsert` settles the row into exactly the shape `findUnchanged` tests for. `UserProvisioningFastPathTest` pins the per-field cases, including that a revoked `admin` claim must not be served from the existing row. + One consequence worth knowing: with a read replica configured, the suspension backstop in `CurrentUser.resolve` reads a row that may be up to the replication lag old. A ban issued through `BanService` is unaffected — it updates `SuspensionRegistry` before writing the row, and the filter refuses the request before it reaches the backstop. What lags is what the registry cannot see: a `suspended_at` edited directly in psql, or a ban issued by another node. - **Channel types.** `PUBLIC` channels are joinable by anyone via `ChannelService.join` **unless archived**. `PRIVATE` channels require `ChannelService.invite` by an admin. The creator becomes the first `ADMIN` member automatically, and `join` promotes the first person into an *empty* channel — otherwise a channel everyone left could never have an admin again. Members can `leave`; the last admin leaving hands the role to the longest-standing remaining member rather than being refused. - **Slug rule.** `Channel.slug` is generated from the name in `ChannelService.create` **and `ChannelService.rename`**: lowercased, non-alphanumerics collapsed to `-`, trimmed to 80 chars. A name with no alphanumerics is rejected. Renaming moves the slug, which is safe because nothing user-facing resolves a channel by slug — pages are `/channels/{id}` and every API route is id-keyed. - **UI icons come from the SVG sprite, never from emoji.** `templates/fragments/icon-sprite.html` holds every symbol; use it as `` (`.icon` = 20px buttons, `.icon-sm` = 14px inline markers), and add a new 24×24 symbol rather than reaching for a glyph. Emoji as icons look wrong for three reasons: they render in the font's own colours so they ignore the theme and can't be dimmed or turned red for a destructive action, they're drawn differently on every platform, and they vanish entirely on hosts with no emoji font. Real emoji stay emoji — reactions, the picker, and custom status are content. Letterforms (`B`, `I`, `S`, `{ }`) in the composer toolbar are labels, not icons; leave them. Note that a message's action row has **one builder and several readers**: `attachActions` in `js/chat/index.js` builds it for channel messages (including server-rendered ones — `channels.html` renders no action row at all) and `js/conversation.js` does the same for DMs. The desktop `⋯` overflow menu and the mobile long-press sheet in `chat-kit.js` both *derive* from the buttons that builder produced, reading their `data-action` and `title`. Add an action in the builder and the other two inherit it; add a second builder and they won't. @@ -127,6 +130,9 @@ Several autoconfigurations that lived inside `spring-boot-autoconfigure` in 3.x - **Sidebar.** `SidebarService.joinedFor(user)` returns a `SidebarView` of **every channel the user is a member of** — not a ranked shortlist — alphabetical (case-insensitive, ties by id), excluding archived ones, rendered as Favourites then Channels. Entries carry `joined` and `favourite`; there is no `admin` flag, because being a channel admin is not something you need in a list you scan fifty times a day (the members panel and the cog show it). The markup is `templates/fragments/sidebar.html`, one copy shared by `channels.html` *and* `conversation.html`, in the **left** `