Skip to content

[srp-client] use proportional jitter for retry timer - #12044

Merged
jwhui merged 1 commit into
openthread:mainfrom
abtink:srpc/adapt-retry-jitter
Oct 20, 2025
Merged

[srp-client] use proportional jitter for retry timer#12044
jwhui merged 1 commit into
openthread:mainfrom
abtink:srpc/adapt-retry-jitter

Conversation

@abtink

@abtink abtink commented Oct 15, 2025

Copy link
Copy Markdown
Member

This change updates the jitter calculation for the SRP client's retry mechanism.

Previously, a fixed jitter value was used. This could lead to synchronized retries from multiple clients, especially as the retry interval grows.

The new implementation calculates the jitter as a fraction of the current retry interval (1/5th), ensuring that the jitter scales with the wait time. This helps to better decorrelate retries from different clients.

A new constant kRetryJitterDivisor is introduced for this calculation. The jitter is clamped to a minimum value given by kRetryIntervalJitter.

@abtink

abtink commented Oct 15, 2025

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a proportional jitter to the SRP client's retry mechanism, which is a good improvement to help decorrelate retries from multiple clients. The implementation is correct. I've added one suggestion to make the code for jitter calculation more concise and robust by initializing the variable at declaration.

Comment thread src/core/net/srp_client.cpp
@github-actions

github-actions Bot commented Oct 15, 2025

Copy link
Copy Markdown

Merging #12044 into main

name branch text data bss total
ot-cli-ftd 64e064f 480776 860 66580 548216
73631de 480792 860 66580 548232
+/- +16 +0 +0 +16
ot-ncp-ftd 64e064f 446700 764 61824 509288
73631de 446732 764 61824 509320
+/- +32 +0 +0 +32
ot-cli-mtd 64e064f 372936 764 51052 424752
73631de 372968 764 51052 424784
+/- +32 +0 +0 +32
ot-ncp-mtd 64e064f 353900 764 46320 400984
73631de 353932 764 46320 401016
+/- +32 +0 +0 +32
ot-cli-ftd-br 64e064f 584200 868 136412 721480
73631de 584232 868 136412 721512
+/- +32 +0 +0 +32
ot-rcp 64e064f 63136 568 20804 84508
73631de 63136 568 20804 84508
+/- +0 +0 +0 +0
Library files
name branch text data bss total
libopenthread-ftd.a 64e064f 247637 95 40358 288090
73631de 247665 95 40358 288118
+/- +28 +0 +0 +28
libopenthread-cli-ftd.a 64e064f 61459 0 8083 69542
73631de 61459 0 8083 69542
+/- +0 +0 +0 +0
libopenthread-ncp-ftd.a 64e064f 33581 0 5948 39529
73631de 33581 0 5948 39529
+/- +0 +0 +0 +0
libopenthread-mtd.a 64e064f 165719 0 24854 190573
73631de 165747 0 24854 190601
+/- +28 +0 +0 +28
libopenthread-cli-mtd.a 64e064f 41414 0 8059 49473
73631de 41414 0 8059 49473
+/- +0 +0 +0 +0
libopenthread-ncp-mtd.a 64e064f 25805 0 5948 31753
73631de 25805 0 5948 31753
+/- +0 +0 +0 +0
libopenthread-ftd-br.a 64e064f 352431 100 110158 462689
73631de 352459 100 110158 462717
+/- +28 +0 +0 +28
libopenthread-cli-ftd-br.a 64e064f 79303 0 8115 87418
73631de 79303 0 8115 87418
+/- +0 +0 +0 +0
libopenthread-rcp.a 64e064f 9944 0 5060 15004
73631de 9944 0 5060 15004
+/- +0 +0 +0 +0
libopenthread-radio.a 64e064f 19411 0 238 19649
73631de 19411 0 238 19649
+/- +0 +0 +0 +0

@codecov

codecov Bot commented Oct 15, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.15%. Comparing base (64e064f) to head (301c7cb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/core/net/srp_client.cpp 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12044      +/-   ##
==========================================
+ Coverage   75.88%   77.15%   +1.26%     
==========================================
  Files         667      666       -1     
  Lines       97424    94615    -2809     
==========================================
- Hits        73932    72998     -934     
+ Misses      23492    21617    -1875     
Files with missing lines Coverage Δ
src/core/net/srp_client.hpp 93.65% <ø> (ø)
src/core/net/srp_client.cpp 93.24% <0.00%> (+0.62%) ⬆️

... and 239 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@abtink
abtink marked this pull request as ready for review October 17, 2025 12:35
@jwhui
jwhui requested a review from Copilot October 18, 2025 04:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

Switch SRP client retry jitter from a fixed value to a proportion of the current retry interval to better decorrelate client retries.

  • Introduce kRetryJitterDivisor (1/5) and compute jitter as retry_interval / kRetryJitterDivisor, with a minimum clamp to kRetryIntervalJitter.
  • Apply the computed jitter in mTimer.Start(Random::NonCrypto::AddJitter(...)).

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/core/net/srp_client.hpp Adds kRetryJitterDivisor constant to control proportional jitter.
src/core/net/srp_client.cpp Computes jitter proportionally to current retry interval and uses it when starting the retry timer.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/core/net/srp_client.cpp
Comment thread src/core/net/srp_client.cpp
Comment thread src/core/net/srp_client.cpp
Comment thread src/core/net/srp_client.hpp
This change updates the jitter calculation for the SRP client's retry
mechanism.

Previously, a fixed jitter value was used. This could lead to
synchronized retries from multiple clients, especially as the retry
interval grows.

The new implementation calculates the jitter as a fraction of the
current retry interval (1/5th), ensuring that the jitter scales with
the wait time. This helps to better decorrelate retries from different
clients.

A new constant `kRetryJitterDivisor` is introduced for this
calculation. The jitter is clamped to a minimum value given by
`kRetryIntervalJitter`.
@abtink
abtink force-pushed the srpc/adapt-retry-jitter branch from 2be323c to 301c7cb Compare October 20, 2025 18:53
@jwhui
jwhui merged commit 4fde897 into openthread:main Oct 20, 2025
102 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants