Second code review round (xhigh) of 057c1fd + 085e609: 5 findings.
Fixed:
- manage_server's subscribe-failure path called reader.unsubscribe() directly;
with the legacy OpcManager a dead session made it raise out of the except
block and stop the main loop. Both paths now go through _unsubscribe_reader,
the single guarded per-reader unsubscribe.
- An unsubscribe failure also increments OPC_SUBSCRIPTION_ERRORS (same tags as
the subscription failures), besides the OPC_UNSUBSCRIBE_ERROR notification.
- The contract test creates the subscriptions through the readers' real
create_subscription (fake pool / client) instead of writing engine
internals, and checks the pool pin/unpin pair.
Documented (no code change):
- If delete() fails while the session is still alive (e.g. a timeout), the
server-side subscription may keep delivering data until that session ends
(reconnection or pod restart); noted in _unsubscribe_reader.
- manage_leases now unsubscribes before releasing the leases: if Redis fails
while releasing them, the slots stay unread until LEASE_TTL expires, which is
preferred to two pods reading the same slot.
284 unit tests; the 2 new behavior tests fail on 085e609.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4235ce8edb