[scd] Time-bound subscription locks and add lock diagnostics

[ashishjsharda]

Problem

Issue #1311 reports high latency and timeouts for SCD operations under overlapping traffic patterns. Lock acquisition in LockSubscriptionsOnCells becomes expensive when many subscriptions share S2 cells, regardless of temporal relevance.

Solution

Narrow lock scope to subscriptions that overlap both the relevant S2 cells and the operation time window, while preserving correctness by always locking explicitly referenced subscription IDs. Add low-noise diagnostics to improve production observability of costly lock queries.

Changes

1. Time-bounded lock scope

Updated LockSubscriptionsOnCells to accept startTime and endTime, locking rows matching:

• cells && $1
• COALESCE(starts_at <= endTime, true)
• COALESCE(ends_at >= startTime, true)
• OR id = ANY(explicitSubscriptionIDs)

2. Call-site updates

Updated OIR mutation paths to pass the relevant time window:

• Delete OIR: uses old.StartTime / old.EndTime
• Upsert OIR: uses validParams.uExtent.StartTime / validParams.uExtent.EndTime

3. Lock diagnostics (thresholded, low-noise)

Warnings emitted when:

• duration >= 500ms, or
• matched_rows >= 1000

Fields logged:

• global_lock
• duration
• matched_rows
• cell_count
• explicit_subscription_id_count

Failure warnings:

• SCD global lock query failed
• SCD subscription lock query failed

4. Documentation

Extended docs/operations/performances.md with diagnostics behavior, fields, and thresholds.

Validation

• go test ./pkg/scd/store/datastore — passed
• go test ./pkg/scd/... — passed locally

Risk / Compatibility

• LockSubscriptionsOnCells interface signature updated; all in-repo call sites patched.
• Cell-based lock scope intentionally narrowed by time overlap; explicit subscription ID locking preserved.
• Diagnostics are passive and thresholded, minimizing steady-state log noise.

Follow-up

• Validate against existing SCD load-test scenario from High Latency for SCD DSS Requests #1311 (p95/p99 latency, timeout/error rates)
• Tune thresholds based on production observations if needed

Closes #1311

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: ashishjsharda / name: Ashish Sharda (590c4fe, 672eec8, a0f3a93)

[barroco]

Thank you very much @ashishjsharda for this contribution.

Please see comments inline.

[barroco]

Thanks for the clarification @ashishjsharda, while looking at it again, I am not sure about the usage of RowsAffected. Can you please confirm the behaviour is correct ?

[ashishjsharda]

Thanks for the clarification @ashishjsharda, while looking at it again, I am not sure about the usage of RowsAffected. Can you please confirm the behaviour is correct ?

Thanks for the follow-up. You’re right to question it.

In the latest update (590c4fe), we no longer rely on RowsAffected() for diagnostics behavior:

• warning trigger is now duration-only (>= 4s),
• row-count-based criteria were removed.

So the behavior of this PR no longer depends on RowsAffected() semantics for SELECT ... FOR UPDATE.

[barroco]

@ashishjsharda many thanks for the changes. I will review them tomorrow first thing. Please note that we are still missing your signature of the Contributor License Agreement (CLA) (See this automatic check) before being able to merge this PR.

[barroco]

Thank you @ashishjsharda for the changes.
Knowing that:

1. in terms of performance, the change is not making the situation worse and best case improves it as far as we have tested
2. and that, the notifyVolume differing from validParams is out of scope of this PR

I am good to merge it. We will release it as patch update upon request so it can be tested in a production environment allowing users to easily rollback if there is any critical issue. Ideally, we will wait for this confirmation before releasing a new minor version including it.

Waiting for all automated checks to pass including the CLA to be signed to merge.

[ashishjsharda]

Thank you @ashishjsharda for the changes. Knowing that:

1. in terms of performance, the change is not making the situation worse and best case improves it as far as we have tested
2. and that, the notifyVolume differing from validParams is out of scope of this PR

I am good to merge it. We will release it as patch update upon request so it can be tested in a production environment allowing users to easily rollback if there is any critical issue. Ideally, we will wait for this confirmation before releasing a new minor version including it.

Waiting for all automated checks to pass including the CLA to be signed to merge.

Thanks for the review and approval, @barroco.
I’ve signed the CLA, so the automated checks should pass now.
Appreciate the feedback on performance and the clarification around notifyVolume vs validParams.