feat(core): extract WAL sender into a Klio-decoupled pkg/sendwal package - #182
Draft
joao00001 wants to merge 4 commits into
Draft
feat(core): extract WAL sender into a Klio-decoupled pkg/sendwal package#182joao00001 wants to merge 4 commits into
joao00001 wants to merge 4 commits into
Conversation
Klio's WAL receiver (core/internal/client/sendwal) implements the PostgreSQL physical replication protocol end to end: negotiating a start position, managing the replication slot lifecycle, streaming WAL via START_REPLICATION, and buffering received bytes. None of that logic is actually Klio-specific, but the package was hard-wired to Klio's own config.Data, the generated gRPC client, and the internal opentelemetry package, which made it unusable outside this module. Move the package to core/pkg/sendwal (plus its buffer and infrastructure subpackages) with the same behavior, but replace the Klio-specific dependencies with two seams a caller supplies: a ReplicationCoordinator interface (negotiate start position, reset the stream, store history files) and a HandlerFactory that builds the buffer.Handler sink for received WAL bytes. Options carries the former config.SourceConfig-derived tunables (slot name, cluster name, buffer size, flush/standby timeouts) as plain fields instead of a config struct. The buffer.Handler interface is kept exactly as it was, without the SyncedOffset() method proposed in klio#124/cloudnative-pg#98, since that change is still being reworked upstream. The Klio-specific timeline gauge metric (opentelemetry.ClientWal.Timeline) is dropped from the generic package, as it depends on Klio's own instrumentation; a caller that still wants it can re-add it around the new seams. This is the first step of the shared Go module extraction discussed in cloudnative-pg#148, kept intentionally small so the package boundary and interface shapes can get feedback before anything grows around them. Klio's own wiring to this package follows in the next commit. Signed-off-by: Joao Detomini <joao.detomini@enterprisedb.com>
core/cmd/send-wal.go and core/cmd/reset-lsn.go still constructed the WAL receiver directly against *grpcclient.Connection and *config.Data, which sendwal.Process no longer accepts after the previous commit. Add the two adapters the new package needs on the Klio side, in core/internal/client/klioclient/grpcclient: - SendWALCoordinator implements sendwal.ReplicationCoordinator against a *Connection, translating RequestStart/ResetStream/StoreHistoryFile into the existing RequestWALStart/ResetWALStream RPCs and StoreHistoryFile call. - KlioClientStreamingHandler (moved from the old buffer/grpc.go) and NewKlioClientHandlerFactory implement sendwal.HandlerFactory, streaming received WAL blocks straight to the Klio server as before. Update both commands to build a coordinator and a handler factory from the existing *grpcclient.Connection, and to pass the DSN, slot, cluster name, buffer size and timeouts through sendwal.Options instead of the old *config.Data. Behavior is unchanged: same RPCs, same streaming handler, same tier2 flag propagation. Signed-off-by: Joao Detomini <joao.detomini@enterprisedb.com>
core/internal/client/sendwal and its buffer/infrastructure subpackages are fully superseded by core/pkg/sendwal plus the Klio-side adapters in grpcclient, wired up in the previous commit. Nothing in the module imports the old path anymore. Signed-off-by: Joao Detomini <joao.detomini@enterprisedb.com>
The two adapters added to wire send-wal/reset-lsn to the extracted sendwal package (SendWALCoordinator and KlioClientStreamingHandler / NewKlioClientHandlerFactory) had no test coverage of their own; the extracted package itself kept the original suite, but the Klio-side plumbing was new. Add sendwal_adapter_test.go and wal_handler_test.go, following this package's existing testing.T/testify style. Both reuse the temporary, local Klio server already set up by ConnectTemporary in connection_test.go, rather than mocking the gRPC client: RequestStart and ResetStream are exercised against the real destination-side negotiation logic (first contact, system ID mismatch, resetting past/before the latest archived WAL), and the handler lifecycle test does a full open/write/close/download round trip to confirm the streamed bytes come back unchanged. Signed-off-by: Joao Detomini <joao.detomini@enterprisedb.com>
joao00001
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
core/internal/client/sendwalimplements the Postgres physical-replication WAL receiver (slot negotiation,START_REPLICATION, timeline history files, standby feedback) but is wired directly to Klio's own gRPC client and configuration types. That coupling makes the receiver unusable outside Klio, which blocks the shared-Go-module direction discussed in #148: other consumers (e.g. a future Barman-in-Go) need the same replication logic without pulling in Klio's client or wire protocol.Change(s)
core/pkg/sendwal, with no remaining dependency on Klio's config, gRPC client, or internal OpenTelemetry setup.ReplicationCoordinator(negotiates the start position, resets the stream, stores timeline history files) andHandlerFactory(builds abuffer.Handlersink per timeline/segment size). Streaming options move intosendwal.Options.SendWALCoordinatorincore/internal/client/klioclient/grpcclient/sendwal_adapter.go, implementingReplicationCoordinatoragainst Klio's existing*Connection.core/internal/client/klioclient/grpcclient/wal_handler.goasKlioClientStreamingHandler, and addNewKlioClientHandlerFactoryimplementingsendwal.HandlerFactory.cmd/send-wal.goandcmd/reset-lsn.goto build the new coordinator/handler-factory pair and callsendwal.New(...).core/internal/client/sendwaltree entirely; no other package referenced it.No behavioral change is intended: the replication protocol, slot handling, and streaming logic are unchanged, only relocated and decoupled.
Testing
go build,go vet, andgo testare clean across the wholecoremodule, not just the touched packages.sendwal_adapter_test.go,wal_handler_test.go), exercising them against a real temporary local Klio server (viaConnectTemporary) rather than mocks, covering start negotiation, system-ID mismatch rejection, stream reset, history file storage, and the streaming handler's open/write/close lifecycle with a round-trip download check.