Skip to content

Ft/instance aware counter#10

Merged
dev-davexoyinbo merged 36 commits into
mainfrom
ft/instance-aware-counter
Apr 3, 2026
Merged

Ft/instance aware counter#10
dev-davexoyinbo merged 36 commits into
mainfrom
ft/instance-aware-counter

Conversation

@dev-davexoyinbo
Copy link
Copy Markdown
Owner

@dev-davexoyinbo dev-davexoyinbo commented Apr 3, 2026

Branch Changes

New Features

Instance-Aware Counters (icounter module)

A fully new counter family that tracks each running instance's contribution separately, with automatic cleanup of dead instances.

  • StrictInstanceAwareCounter — immediately consistent; every inc, set, get, del, and clear executes atomically against Redis via Lua scripts. Tracks per-instance counts in a Redis hash, uses an epoch-based mechanism to detect stale contributions, and runs a background heartbeat task to keep the instance registered as live. Dead instances (no heartbeat for dead_instance_threshold_ms, default 30 s) are automatically removed from the cumulative total by the next live instance that touches the same key.

  • LaxInstanceAwareCounter — eventual consistency; inc/dec deltas are buffered locally and flushed in batches to the backing StrictInstanceAwareCounter every flush_interval (default 20 ms). Epoch-bumping operations (set, del, clear) flush any pending delta first, then delegate immediately to the strict counter.

  • InstanceAwareCounterTrait — shared async interface with 10 methods: instance_id, inc, dec, set, set_on_instance, get, del, del_on_instance, clear, clear_on_instance.

  • ActivityTracker — internal utility that tracks whether a counter is actively being used, allowing background tasks (heartbeat, flush) to sleep when idle and wake immediately on activity.

  • Recovery logic — when StrictInstanceAwareCounter's heartbeat detects its instance was cleaned up as dead while offline (e.g. GC pause or network blip), it recovers its contributions epoch-safely via pipelined INC_IF_EPOCH_MATCHES Lua calls.


Breaking Changes

Counter facade removed

The Counter enum/facade has been deleted. StrictCounter and LaxCounter are now created directly from CounterOptions:

// Before
let counter = Counter::new(options);
let strict = counter.strict();
let lax    = counter.lax();


// After
let counter = StrictCounter::new(options.clone());
let lax     = LaxCounter::new(options);

CounterOptions now implements Clone.

dec added to InstanceAwareCounterTrait

All instance-aware counter implementations expose a dec method that delegates to inc(key, -count).

StrictCounter::new returns Arc<StrictCounter>

Previously returned a plain value.


Documentation

  • README.md and docs/lib.md rewritten: updated quick-start examples, unified 4-column comparison table covering all four counter types (StrictCounter, LaxCounter, StrictInstanceAwareCounter, LaxInstanceAwareCounter), and full usage examples for both instance-aware counter types including dec.

  • Inline # Examples added to every public method on all four counter types and InstanceAwareCounterTrait — 57 doc tests total, all executed (not no_run) against a real Redis instance.

  • src/__doctest_helpers.rs — a #[doc(hidden)] module providing shared async setup helpers (strict_counter, lax_counter, strict_icounter, two_strict_icounters, lax_icounter, two_lax_icounters). Each generates a UUID-namespaced Redis prefix so concurrent doc tests never share state. The Redis URL is read from $REDIS_URL, defaulting to redis://127.0.0.1:6379.


Infrastructure

  • GitHub Actions workflow (.github/workflows/workflow.yml) added.
  • Makefile updated with redis-up, redis-down, test, and bench targets backed by Docker Compose.
  • instance-aware-counter feature flag added to Cargo.toml; enabled by default.
  • Test suite expanded: 115 unit tests + 57 doc tests (172 total).

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Add instance-aware counters with per-instance tracking and automatic dead-instance cleanup

✨ Enhancement 🧪 Tests 📝 Documentation

Grey Divider

Walkthroughs

Description
• **Instance-aware counters**: Introduces two new counter types (StrictInstanceAwareCounter and
  LaxInstanceAwareCounter) that track per-instance contributions separately with automatic
  dead-instance cleanup via epoch-based mechanisms and Lua scripts
• **Strict instance-aware counter**: Fully consistent counter with immediate atomic operations,
  background heartbeat task for dead-instance detection, and recovery logic via pipelined
  INC_IF_EPOCH_MATCHES_LUA calls
• **Lax instance-aware counter**: Eventually-consistent counter that buffers inc/dec deltas
  locally and flushes them in batches (default 20 ms) to the backing strict counter
• **Shared trait**: New InstanceAwareCounterTrait with 10 async methods (instance_id, inc,
  dec, set, set_on_instance, get, del, del_on_instance, clear, clear_on_instance)
• **Activity tracking utility**: New ActivityTracker for coordinating background flush task
  sleep/wake cycles, replacing custom epoch-change logic in LaxCounter
• **Breaking changes**: Removed Counter facade; StrictCounter and LaxCounter now constructed
  directly from CounterOptions; StrictCounter::new() now returns Arc<StrictCounter>
• **Comprehensive documentation**: Rewritten README.md and docs/lib.md with unified 4-column
  comparison table, full usage examples for all counter types, and 57 executable doc tests
• **Test coverage**: 115 unit tests for instance-aware counters plus 57 doc tests (172 total),
  covering single/multi-instance scenarios, epoch propagation, dead-instance cleanup, and recovery
  logic
• **Infrastructure**: Added GitHub Actions CI workflow with Redis service, updated Makefile with
  Docker Compose targets, added instance-aware-counter feature flag (enabled by default), and uuid
  dependency
Diagram
flowchart LR
  A["CounterOptions"] -->|"new()"| B["StrictCounter<br/>Arc wrapped"]
  A -->|"new()"| C["LaxCounter<br/>buffered deltas"]
  A -->|"new()"| D["StrictInstanceAwareCounter<br/>per-instance tracking"]
  A -->|"new()"| E["LaxInstanceAwareCounter<br/>buffered + per-instance"]
  D -->|"implements"| F["InstanceAwareCounterTrait"]
  E -->|"implements"| F
  D -->|"uses"| G["Lua scripts<br/>epoch-safe ops"]
  D -->|"uses"| H["Heartbeat task<br/>dead-instance cleanup"]
  E -->|"flushes to"| D
  E -->|"uses"| I["ActivityTracker<br/>flush coordination"]
  C -->|"uses"| I
Loading

Grey Divider

File Changes

1. src/icounter/strict_instance_aware_counter.rs ✨ Enhancement +1354/-0

Instance-aware counter with epoch-safe recovery and dead-instance cleanup

• Implements StrictInstanceAwareCounter, a fully-consistent instance-aware distributed counter
 with per-instance contribution tracking and automatic dead-instance cleanup via Redis Lua scripts
• Provides 10 core methods (inc, dec, set, set_on_instance, get, del, del_on_instance,
 clear, clear_on_instance) with epoch-based staleness detection and recovery logic
• Includes background heartbeat task that detects when an instance was cleaned up as dead and
 recovers its contributions via pipelined INC_IF_EPOCH_MATCHES_LUA calls
• Implements InstanceAwareCounterTrait and supports batched increment operations via inc_batch()
 method

src/icounter/strict_instance_aware_counter.rs


2. src/icounter/tests/strict_instance_aware_counter.rs 🧪 Tests +970/-0

Comprehensive test suite for strict instance-aware counter

• Adds 115 comprehensive unit tests covering single-instance basics, N-instance cumulative accuracy,
 epoch propagation, complex interleaved operations, dead-instance scenarios, and recovery logic
• Tests verify correct behavior across multiple instances, epoch bumping, stale detection, and
 recovery paths (via inc, get, and mark_alive)
• Includes edge cases like live-instance boundary conditions, multi-key independence, and epoch
 isolation between keys

src/icounter/tests/strict_instance_aware_counter.rs


3. src/icounter/tests/lax_instance_aware_counter.rs 🧪 Tests +214/-0

Test suite for lax instance-aware counter with flush verification

• Adds 14 tests for LaxInstanceAwareCounter covering buffered increments, flush behavior,
 dead-instance cleanup through the lax wrapper, and clear/clear_on_instance operations
• Tests verify that pending deltas are flushed before epoch-changing operations (set, del) and
 that multiple instances cumulate correctly after background flush
• Validates dead-instance cleanup is triggered through the lax wrapper and that instance-specific
 clearing preserves other instances' contributions

src/icounter/tests/lax_instance_aware_counter.rs


View more (21)
4. src/counter/lax_counter.rs Refactoring +165/-77

Refactor lax counter to use shared ActivityTracker utility

• Replaces custom epoch-change and activity-tracking logic with shared ActivityTracker utility,
 simplifying the flush task implementation
• Removes epoch, last_commited_epoch, is_active_watch, and epoch_change_task() in favor of
 centralized activity tracking via activity.signal() and activity.get_is_active()
• Updates RedisKeyGeneratorTypeKey::LaxCounter to RedisKeyGeneratorTypeKey::Lax for consistency
• Adds comprehensive doc comments with examples to all public methods (inc, dec, get, set,
 del, clear)

src/counter/lax_counter.rs


5. src/counter/mod.rs 📝 Documentation +20/-33

Update counter module documentation and remove common module

• Removes mod common declaration (moved to lib.rs) and updates module documentation to reflect
 removal of Counter facade
• Adds doc comment with example to CounterOptions::new() demonstrating construction with Redis
 connection
• Clarifies that StrictCounter and LaxCounter are constructed directly from CounterOptions

src/counter/mod.rs


6. src/lib.rs ✨ Enhancement +7/-1

Add instance-aware counter module and doctest helpers

• Adds new icounter module behind instance-aware-counter feature flag (enabled by default)
• Moves mod common outside of feature gate so it is always available
• Adds conditional __doctest_helpers module for doc tests when both counter and
 instance-aware-counter features are enabled

src/lib.rs


7. src/icounter/lax_instance_aware_counter.rs ✨ Enhancement +705/-0

Buffered instance-aware counter with local delta accumulation

• New file implementing LaxInstanceAwareCounter, a buffered instance-aware counter that
 accumulates inc deltas locally and flushes them in bulk to a backing StrictInstanceAwareCounter
• Implements InstanceAwareCounterTrait with 10 async methods including inc, dec, set,
 set_on_instance, get, del, del_on_instance, clear, and clear_on_instance
• Includes background flush task that collects stale deltas and batches them to Redis every
 flush_interval (default 20 ms)
• Provides comprehensive doc tests with examples for all public methods

src/icounter/lax_instance_aware_counter.rs


8. src/icounter/mod.rs ✨ Enhancement +267/-0

Instance-aware counter trait and module organization

• New module defining InstanceAwareCounterTrait with 10 async methods for instance-aware counter
 operations
• Trait methods include instance_id, inc, dec, set, set_on_instance, get, del,
 del_on_instance, clear, clear_on_instance
• Each method includes comprehensive doc tests demonstrating usage with real Redis instances
• Exports StrictInstanceAwareCounter and LaxInstanceAwareCounter implementations

src/icounter/mod.rs


9. src/counter/counter_trait.rs 📝 Documentation +100/-0

Documentation examples for counter trait methods

• Added doc test examples to all 7 methods in CounterTrait (inc, dec, get, set, del,
 clear)
• Examples demonstrate usage patterns including negative counts, non-existent keys, and batch
 operations
• All examples are executable against a real Redis instance via doctest helpers

src/counter/counter_trait.rs


10. src/icounter/tests/common.rs 🧪 Tests +131/-0

Test helpers for instance-aware counter tests

• New test utilities module for instance-aware counter tests
• Provides helper functions to create single counters, pairs of counters, and n-tuples of counters
 sharing Redis prefixes
• Includes functions with configurable dead_instance_threshold_ms for testing cleanup behavior
• Uses unique prefixes based on run ID to prevent test interference

src/icounter/tests/common.rs


11. src/__doctest_helpers.rs 🧪 Tests +93/-0

Shared doctest setup helpers for all counter types

• New module providing shared async setup helpers for doc tests across all counter types
• Exports 6 helper functions: strict_counter, lax_counter, strict_icounter,
 two_strict_icounters, lax_icounter, two_lax_icounters
• Each helper generates UUID-namespaced Redis prefixes to prevent concurrent doc test state sharing
• Reads REDIS_URL environment variable with fallback to redis://127.0.0.1:6379

src/__doctest_helpers.rs


12. src/common/activity_tracker.rs ✨ Enhancement +111/-0

Activity tracker for background flush task coordination

• New internal utility tracking counter activity to drive flush task sleep/wake cycles
• Uses epoch counter incremented on background timer and watch channel for broadcasting
• Provides signal() method to wake subscribers when epoch advances and get_is_active() to check
 activity status
• Spawns background tasks for epoch advancement and activity state management

src/common/activity_tracker.rs


13. src/counter/strict_counter.rs ✨ Enhancement +23/-4

Arc-wrapped return type and documentation for strict counter

• Modified StrictCounter::new() to return Arc<StrictCounter> instead of plain value
• Added comprehensive doc test example showing counter creation and basic usage
• Updated RedisKeyGeneratorTypeKey from StrictCounter to Strict for consistency

src/counter/strict_counter.rs


14. src/common/mod.rs ✨ Enhancement +12/-2

Activity tracking and instance-aware counter key types

• Added ActivityTracker module and constant EPOCH_CHANGE_INTERVAL (15 seconds)
• Extended RedisKeyGeneratorTypeKey enum with InstanceAware and LaxInstanceAware variants
• Renamed existing enum variants from StrictCounter/LaxCounter to Strict/Lax for consistency

src/common/mod.rs


15. src/counter/tests/common.rs 🧪 Tests +1/-1

Counter test helper return type update

• Updated make_strict_counter() return type from StrictCounter to Arc<StrictCounter> to match
 new API

src/counter/tests/common.rs


16. src/icounter/tests/mod.rs 🧪 Tests +3/-0

Instance-aware counter test module structure

• New test module organization file for instance-aware counter tests
• Exports common module and submodules for strict_instance_aware_counter and
 lax_instance_aware_counter tests

src/icounter/tests/mod.rs


17. docs/lib.md 📝 Documentation +225/-35

Comprehensive documentation rewrite with instance-aware counters

• Completely rewritten documentation with expanded feature descriptions and new instance-aware
 counter section
• Updated quick-start examples to show direct StrictCounter and LaxCounter construction instead
 of Counter facade
• Added comprehensive 6-column comparison table covering all four counter types with consistency,
 latency, I/O, and use case details
• Added full sections on StrictInstanceAwareCounter and LaxInstanceAwareCounter with code
 examples and dead-instance cleanup explanation
• Updated rate limiting section with complete Redis-backed and hybrid provider examples

docs/lib.md


18. README.md 📝 Documentation +251/-32

README update with instance-aware counters and expanded examples

• Updated feature description to include instance-aware counters alongside strict and lax counters
• Added instance-aware counter feature flag to feature table
• Updated quick-start example to show direct counter construction instead of Counter facade
• Added new "Choosing a counter" section with 8-column comparison table for all four counter types
• Added comprehensive "Instance-aware counters" section with StrictInstanceAwareCounter,
 dead-instance cleanup, and LaxInstanceAwareCounter examples
• Expanded rate limiting section with local absolute, Redis-backed, and hybrid provider code
 examples

README.md


19. .github/workflows/workflow.yml Configuration +51/-0

GitHub Actions CI workflow with Redis service

• New GitHub Actions workflow for continuous integration
• Sets up Redis 7.2 service with health checks on port 6379
• Installs Rust toolchain and runs tests with --all-features flag
• Includes Redis readiness check with retry logic before running tests

.github/workflows/workflow.yml


20. Cargo.toml Dependencies +3/-1

Feature flag and dependency additions for instance-aware counters

• Added instance-aware-counter feature flag (enabled by default alongside counter)
• Added uuid dependency with v4 feature for generating instance IDs
• Updated default features to include both counter and instance-aware-counter
• Updated test command in Makefile to use --all-features flag

Cargo.toml


21. Makefile Configuration +2/-5

Makefile test and bench target updates

• Updated test target to run with --all-features flag and --show-output for test output
 visibility
• Updated bench target to run with --all-features flag
• Removed test-example target

Makefile


22. .env Configuration +1/-0

Environment variable for Redis connection

• Added REDIS_URL environment variable set to redis://localhost:16379 for local development

.env


23. src/.DS_Store Miscellaneous +0/-0

macOS directory metadata file

• macOS system file (binary metadata for directory display)

src/.DS_Store


24. src/counter/common.rs Additional files +0/-3

...

src/counter/common.rs


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Apr 3, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

Grey Divider


Action required

1. Flush wakeups can be missed 🐞 Bug ☼ Reliability
Description
ActivityTracker::signal only notifies watchers when the background epoch advances, so activity
occurring after the counter becomes inactive but before the next epoch tick may never wake the flush
task. This can leave buffered deltas unflushed indefinitely (until another later activity happens to
cross an epoch boundary).
Code

src/common/activity_tracker.rs[R91-100]

+    /// Sends an epoch-change signal if the epoch has advanced since the last
+    /// signal, waking any subscribers (e.g. the flush task).
+    #[inline(always)]
+    pub(crate) fn signal(&self) {
+        let epoch = self.epoch.load(Ordering::Relaxed);
+        if self.last_commited_epoch.load(Ordering::Relaxed) < epoch {
+            let _ = self.is_active_watch.send(epoch);
+            self.last_commited_epoch.store(epoch, Ordering::Relaxed);
+        }
+    }
Evidence
signal() is gated by epoch advancement; meanwhile both LaxCounter and LaxInstanceAwareCounter flush
loops block on watch.changed() whenever ActivityTracker reports inactive. With EPOCH_CHANGE_INTERVAL
at 15s and the inactive state reached after epoch_interval/2, there is a 7.5s window where new
writes can be buffered but never wake the flush loop, so no Redis flush occurs if no further
activity follows.

src/common/activity_tracker.rs[91-100]
src/counter/lax_counter.rs[154-179]
src/icounter/lax_instance_aware_counter.rs[188-209]
src/common/mod.rs[7-11]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ActivityTracker::signal()` only calls `watch::Sender::send()` when `last_commited_epoch < epoch`. If the counter becomes inactive (flush task goes into `changed().await`) and then a new operation calls `signal()` within the same epoch value, no watch notification is sent and the flush loop can remain blocked forever (no timeout), so buffered deltas may never be flushed.

### Issue Context
Both `LaxCounter` and `LaxInstanceAwareCounter` rely on `ActivityTracker` to wake their background flush tasks.

### Fix Focus Areas
- src/common/activity_tracker.rs[91-100]
- src/counter/lax_counter.rs[154-179]
- src/icounter/lax_instance_aware_counter.rs[188-209]

### Suggested fix
Make `signal()` always wake subscribers when called (e.g., always `send()`/`send_modify()`), or at minimum ensure it *always* sends when currently inactive. Consider also setting `is_active = true` directly inside `signal()` so the active state reflects activity immediately.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Negative recovery skipped 🐞 Bug ≡ Correctness
Description
StrictInstanceAwareCounter only recovers prior contributions when old_local_count > 0, but the API
supports decrements via negative deltas. If an instance is cleaned up as dead while holding a
negative contribution, recovery will not restore that contribution and cumulative totals become
incorrect.
Code

src/icounter/strict_instance_aware_counter.rs[R965-973]

+        let instance_created = instance_created_raw != 0;
+        let should_recover = instance_created && local_epoch == redis_epoch;
+
+        let old_local_count = self.get_local_count(key);
+        self.update_local_store(key, redis_epoch, cumulative, inst_count);
+
+        if should_recover && old_local_count > 0 {
+            return Box::pin(self.inc(key, old_local_count)).await;
+        }
Evidence
The recovery condition uses old_local_count > 0, and mark_alive recovery filters count > 0,
which excludes negative contributions. The trait explicitly supports decrements by calling inc with
a negative i64, so negative contributions are a valid state that must be recovered just like
positive ones.

src/icounter/strict_instance_aware_counter.rs[965-973]
src/icounter/strict_instance_aware_counter.rs[870-886]
src/icounter/mod.rs[71-92]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Recovery paths in `StrictInstanceAwareCounter` only restore contributions when `old_local_count > 0` / `count > 0`. This drops valid negative contributions (from `dec` / `inc` with negative delta) after dead-instance cleanup, causing the cumulative total to be wrong.

### Issue Context
The instance-aware API supports negative deltas (`dec` is `inc(key, -count)`), so `local_count` can legitimately be negative.

### Fix Focus Areas
- src/icounter/strict_instance_aware_counter.rs[965-973]
- src/icounter/strict_instance_aware_counter.rs[1124-1132]
- src/icounter/strict_instance_aware_counter.rs[870-886]

### Suggested fix
Change recovery guards from `> 0` to `!= 0` (or `count != 0`) in all recovery-related locations so negative contributions are restored as well. Ensure batched recovery includes negative counts too.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Committed .DS_Store file 🐞 Bug ⚙ Maintainability
Description
A macOS Finder metadata file (src/.DS_Store) was added to the repository, which is unrelated to the
library and will create noisy diffs/merge conflicts. This should not be tracked in git.
Code

src/.DS_Store[R1-5]

+����Bud1���������������
+�����������������������������������������������������������������n�t�e�rbwsp�������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������
+������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������� �������@�������������������������������������������������� �������@������������������������������������������������������ �������@������������������������������������������������������ �������@������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������i�c�o�u�n�t�e�rbwspblob����bplist00�����������
+�]ShowStatusBar[ShowToolbar[ShowTabView_��ContainerShowSidebar\WindowBounds[ShowSidebar�	�	_��{{1260, 723}, {920, 464}}	��#/;R_klmno����������������
+���������������������i�c�o�u�n�t�e�rlsvCblob���)bplist00���������	
Evidence
The PR adds a binary .DS_Store file under src/, which is an OS-generated artifact and not source
code.

src/.DS_Store[1-1]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`src/.DS_Store` is an OS-generated macOS artifact that should not be committed.

### Issue Context
These files frequently change locally and cause unnecessary diffs and merge conflicts.

### Fix Focus Areas
- src/.DS_Store[1-1]

### Suggested fix
Remove `src/.DS_Store` from the repository and add `.DS_Store` to `.gitignore` to prevent future commits.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment on lines +91 to +100
/// Sends an epoch-change signal if the epoch has advanced since the last
/// signal, waking any subscribers (e.g. the flush task).
#[inline(always)]
pub(crate) fn signal(&self) {
let epoch = self.epoch.load(Ordering::Relaxed);
if self.last_commited_epoch.load(Ordering::Relaxed) < epoch {
let _ = self.is_active_watch.send(epoch);
self.last_commited_epoch.store(epoch, Ordering::Relaxed);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Flush wakeups can be missed 🐞 Bug ☼ Reliability

ActivityTracker::signal only notifies watchers when the background epoch advances, so activity
occurring after the counter becomes inactive but before the next epoch tick may never wake the flush
task. This can leave buffered deltas unflushed indefinitely (until another later activity happens to
cross an epoch boundary).
Agent Prompt
### Issue description
`ActivityTracker::signal()` only calls `watch::Sender::send()` when `last_commited_epoch < epoch`. If the counter becomes inactive (flush task goes into `changed().await`) and then a new operation calls `signal()` within the same epoch value, no watch notification is sent and the flush loop can remain blocked forever (no timeout), so buffered deltas may never be flushed.

### Issue Context
Both `LaxCounter` and `LaxInstanceAwareCounter` rely on `ActivityTracker` to wake their background flush tasks.

### Fix Focus Areas
- src/common/activity_tracker.rs[91-100]
- src/counter/lax_counter.rs[154-179]
- src/icounter/lax_instance_aware_counter.rs[188-209]

### Suggested fix
Make `signal()` always wake subscribers when called (e.g., always `send()`/`send_modify()`), or at minimum ensure it *always* sends when currently inactive. Consider also setting `is_active = true` directly inside `signal()` so the active state reflects activity immediately.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +965 to +973
let instance_created = instance_created_raw != 0;
let should_recover = instance_created && local_epoch == redis_epoch;

let old_local_count = self.get_local_count(key);
self.update_local_store(key, redis_epoch, cumulative, inst_count);

if should_recover && old_local_count > 0 {
return Box::pin(self.inc(key, old_local_count)).await;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Negative recovery skipped 🐞 Bug ≡ Correctness

StrictInstanceAwareCounter only recovers prior contributions when old_local_count > 0, but the API
supports decrements via negative deltas. If an instance is cleaned up as dead while holding a
negative contribution, recovery will not restore that contribution and cumulative totals become
incorrect.
Agent Prompt
### Issue description
Recovery paths in `StrictInstanceAwareCounter` only restore contributions when `old_local_count > 0` / `count > 0`. This drops valid negative contributions (from `dec` / `inc` with negative delta) after dead-instance cleanup, causing the cumulative total to be wrong.

### Issue Context
The instance-aware API supports negative deltas (`dec` is `inc(key, -count)`), so `local_count` can legitimately be negative.

### Fix Focus Areas
- src/icounter/strict_instance_aware_counter.rs[965-973]
- src/icounter/strict_instance_aware_counter.rs[1124-1132]
- src/icounter/strict_instance_aware_counter.rs[870-886]

### Suggested fix
Change recovery guards from `> 0` to `!= 0` (or `count != 0`) in all recovery-related locations so negative contributions are restored as well. Ensure batched recovery includes negative counts too.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@dev-davexoyinbo dev-davexoyinbo merged commit 3e28dad into main Apr 3, 2026
1 check passed
@dev-davexoyinbo dev-davexoyinbo deleted the ft/instance-aware-counter branch April 13, 2026 00:37
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