[pull] main from hookdeck:main - #180
Merged
Merged
Conversation
The event-delivery-log guide was a TODO stub, and CLICKHOUSE_LOG_RETENTION_TTL_DAYS was undocumented despite being the only retention mechanism Outpost implements. Covers what the log stores, when an event enters it, how the PostgreSQL and ClickHouse backends differ as the log grows, ClickHouse TTL, and self-managed retention for PostgreSQL. Adds the retention variable to the configuration reference. Refs #1027 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(test): pin ClickHouse merges in the chlogstore dedup tests
TestEventDedup injects six "legacy duplicate" event rows that reuse the
originals' (event_time, event_id) and then asserts a raw count of 9. events is
a ReplacingMergeTree keyed on exactly that pair, so a background merge collapses
the nine rows to three whenever the server decides to run one. Nothing between
the last batch.Send() and the SELECT holds it off, which makes the assertion a
race against the merge scheduler:
Send() -> SELECT -> merge => 9 pass
Send() -> merge -> SELECT => 3 fail
Observed as `expected 0x9, got 0x3` during a full-suite run; passes in
isolation, because every test shares one ClickHouse server and only under load
does the SELECT sit long enough for a merge to land first.
The unmerged state is the one worth asserting on: production never reads with
FINAL (chlogstore/README.md), which is why the read path dedups client-side at
all. So the fix is to stop merges rather than force them — OPTIMIZE FINAL would
delete the very condition under test.
stopMerges() qualifies the table with currentDatabase(). The bare form applies
server-wide and would break the conformance harness, whose FlushWrites calls
OPTIMIZE TABLE ... FINAL; each test already gets its own test_<random> database,
dropped on cleanup, so a qualified stop is scoped to the calling test.
SYSTEM STOP MERGES accepts a nonexistent table without error (verified on
24.10.4.191), so the helper checks system.tables first — otherwise a wrong name
silently restores the flake.
TestFetchAndDedupTruncation has the same dependency and is pinned too. It
inserts evt-trunc-a twice at one timestamp to force a short first batch; a merge
there does not fail the test, it makes the LessOrEqual assertion vacuous and the
overshoot path goes unexercised.
Verified: package green including conformance, dedup tests 10x clean, and with
merges stopped an explicit OPTIMIZE returns `code: 236, Cancelled merging parts`
with the duplicate rows still present.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(chlogstore): derive the stopped table from the log store itself
Follow-up to the merge-race fix. stopMerges was called with a "events" string
literal, so the name could drift from the one the code under test queries and
SYSTEM STOP MERGES would report success while doing nothing.
NewLogStore already derives the name as prefix + "events" and keeps it on
logStoreImpl.eventsTable. The tests are in-package, so they now assert the
concrete type and pass that field — the same value the queries use, prefix
handling included. TestFetchAndDedupTruncation's buildEventQuery call had the
same literal and now uses the field too.
The system.tables check stays as a backstop, since a silently-accepted unknown
table would restore the race without failing anything.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(chlogstore): trim the stopMerges comments
Keep what the code cannot say — why the table is qualified, and that STOP MERGES
is silent on an unknown table — and drop the rest.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(logmq): bound BatchProcessor.Shutdown batcher.Shutdown can deadlock: it stops the ticker, then calls processQueue, whose defer restarts it. If a tick lands between the processingMutex acquisition and the send on the shutdown channel, the ticker goroutine blocks on that mutex forever and the send never finds a receiver. Both goroutines are stuck for the process lifetime. Shutdown now runs the drain on its own goroutine and gives up after 60s, above the legitimate worst case of a 30s final insert plus one emitTimeout drain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(logmq): bound Shutdown in tests, revert the production change Keep the production drain unbounded. The hang is rare enough that changing shutdown behavior to defend against it is not worth it; the only real cost is a 10m CI timeout, so bound it where that cost lands. shutdownBounded fails the test after 30s instead. The package runs in a few seconds, so nothing legitimate is near that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The events/attempts time columns are DateTime64(3). ClickHouse rejects a TTL expression evaluating to DateTime64 until 25.7 (ClickHouse#80710), so setting CLICKHOUSE_LOG_RETENTION_TTL_DAYS crash-loops the service at startup on 24.8 LTS through 25.6, including 25.3 LTS. toDateTime() is accepted by all versions. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore(release): pin GoReleaser and migrate off deprecated config The release workflow ran `goreleaser-action@v4` with `version: latest`, so every tag push built with whatever GoReleaser shipped that morning — v1.2.0 happened to use 2.17.1, unchosen. `goreleaser check` also failed on three deprecations, meaning a future release could break with no warning and nowhere to catch it but a tag push. - Pin to v2.17.1 (the version v1.2.0 actually released with) and bump the action to v6. - archives: `format` -> `formats`. Drops the windows->zip `format_overrides`, which was dead — builds are `goos: [linux]` only, so it could never fire. - dockers + docker_manifests -> dockers_v2. One buildx build now covers both platforms and pushes the manifest in a single step, so the Dockerfile copies binaries from `$TARGETPLATFORM/`. `sbom: false` keeps the published manifest to the same two platform entries as today; SBOM attestations would show up as extra "unknown/unknown" platforms on Docker Hub. - Drop DOCKER_CLI_EXPERIMENTAL, only needed by the old docker_manifests path. `goreleaser check` is clean and `goreleaser release --snapshot` builds both images: correct architectures, version ldflags stamped, all binaries and the entrypoint present. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(release): disable buildx provenance attestations `sbom: false` alone was not enough. Verified by pushing the same context to a scratch repo: the manifest came back with linux/amd64, linux/arm64, and two `unknown/unknown` entries. GoReleaser never passes `--provenance`, so buildx applies its default of attaching provenance whenever it pushes, independent of the `sbom` setting. With `--provenance=false` the pushed manifest is exactly the two platform entries, matching what dockers/docker_manifests publishes today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(release): trim the attestation comment Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Outpost never uses msgpack request binding, but gin links its codec unconditionally. gin guards it behind the nomsgpack build tag, so setting the tag drops github.com/ugorji/go from the binary. Reduces outpost-server from 72.8 MB to 66.5 MB (-8.7%), measured by building both ways with release ldflags. Applied to every build path so release, make and Docker binaries stay consistent. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore(release): publish signed images with SBOM and provenance Turns on the two attestations dockers_v2 can emit and signs the published images with cosign, keyless via the release workflow's OIDC token. - sbom: true and buildx's default provenance, reversing the suppressions added while migrating to dockers_v2 to hold the artifact shape steady. - docker_signs with --key omitted, which is what selects keyless signing. - id-token: write on the job so cosign can exchange the OIDC token for a short-lived certificate. Declaring permissions replaces the defaults, so contents: write is listed explicitly for the GitHub release. Signing runs in the publish phase, so local `--snapshot` builds never invoke cosign. Verified: a full snapshot succeeds on a machine with no cosign at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(release): sign checksums.txt Signs the checksums file with cosign, which covers every archive transitively. This is the artifact form supply-chain tooling looks for — OpenSSF Scorecard's Signed-Releases check inspects release assets for .sigstore.json and similar, and does not see container signatures. Unlike docker_signs, this pipe runs before publish, so it executes during snapshot builds too and fails when cosign is absent. The two Makefile targets that build snapshots locally now pass --skip=sign; verified a full snapshot still succeeds with no cosign installed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )