Skip to content

Use UNLINK for cohort cleanup and enable lazy expiry in examples - #46

Draft
adrianliu0815 wants to merge 1 commit into
amplitude:mainfrom
adrianliu0815:codex/async-cohort-cleanup
Draft

adrianliu0815 wants to merge 1 commit into
amplitude:mainfrom
adrianliu0815:codex/async-cohort-cleanup

Conversation

@adrianliu0815

Copy link
Copy Markdown
Contributor

Large cohort cleanup can block unrelated Redis requests even after streamed diff removes SDIFFSTORE: explicit deletion still uses DEL, and retired or abandoned member sets still expire through Redis.

This change uses UNLINK for explicit cohort member/blob deletion and legacy diff temporary-key cleanup in both standalone and cluster clients. It preserves the previous-version read grace period, publication ordering, and pending-version TTLs. The Docker Compose example enables lazyfree-lazy-expire=yes; the README documents persistent server configuration, managed Redis, failover nodes, ACL requirements, and memory monitoring.

Existing deployments still need lazyfree-lazy-expire=yes on their Redis servers to address TTL expiry stalls. An SDK upgrade alone does not configure those servers. The proxy does not issue CONFIG SET, remove old versions immediately, or add a process-local cleanup timer. UNLINK requires Redis 4.0+ and command permission. The legacy diff algorithm itself is unchanged.

Validation:

  • ./gradlew test :core:ktlintCheck passed with Java 17: 130 tests, no failures or skipped tests.
  • Regression tests cover both diff modes, old-version retention, immediate explicit deletion, membership cleanup, and failed-diff temporary cleanup.
  • Integration tests passed against disposable standalone Redis and a three-primary Redis Cluster. They check real UNLINK calls, immediate key absence, repeated deletion, and TTL cleanup increasing lazyfreed_objects with asynchronous expiry enabled.
  • CI now runs the standalone Redis integration tests with the documented setting. Cluster tests opt in through REDIS_CLUSTER_TEST_URI; standalone tests use REDIS_TEST_URI. These must point to disposable test instances.

The integration tests verify cleanup behavior; they do not establish a production latency bound.

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