BB-673 Fix retry for replication and notification #2633

nicolas2bert · 2025-03-25T16:59:05Z

No description provided.

bert-e · 2025-03-25T16:59:08Z

Hello nicolas2bert,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2025-03-25T16:59:15Z

Incorrect fix version

The Fix Version/s in issue BB-673 contains:

None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.0.5
9.1.0

Please check the Fix Version/s of BB-673, or the target
branch of this pull request.

Kerkesni · 2025-03-25T17:03:56Z

extensions/replication/queueProcessor/QueueProcessor.js

+     * @param {Object} notifConfig.queueProcessor.retry - Object keyed by location type
+     * (e.g., `aws_s3`, `azure`, `gcp`, `scality`), defining retry parameters for each location type.
+     * @param {number} notifConfig.queueProcessor.retry.<locationType>.maxRetries - Maximum number of retry attempts.
+     * @param {number} notifConfig.queueProcessor.retry.<locationType>.timeoutS - Timeout in seconds before giving up.
+     * @param {Object} notifConfig.queueProcessor.retry.<locationType>.backoff - Exponential backoff configuration.
+     * @param {number} notifConfig.queueProcessor.retry.<locationType>.backoff.min - Minimum backoff interval in ms.
+     * @param {number} notifConfig.queueProcessor.retry.<locationType>.backoff.max - Maximum backoff interval in ms.
+     * @param {number} notifConfig.queueProcessor.retry.<locationType>.backoff.jitter - Jitter factor.
+     * @param {number} notifConfig.queueProcessor.retry.<locationType>.backoff.factor - Exponential backoff multiplier.


Suggested change

* @param {Object} notifConfig.queueProcessor.retry - Object keyed by location type

* (e.g., `aws_s3`, `azure`, `gcp`, `scality`), defining retry parameters for each location type.

* @param {number} notifConfig.queueProcessor.retry.<locationType>.maxRetries - Maximum number of retry attempts.

* @param {number} notifConfig.queueProcessor.retry.<locationType>.timeoutS - Timeout in seconds before giving up.

* @param {Object} notifConfig.queueProcessor.retry.<locationType>.backoff - Exponential backoff configuration.

* @param {number} notifConfig.queueProcessor.retry.<locationType>.backoff.min - Minimum backoff interval in ms.

* @param {number} notifConfig.queueProcessor.retry.<locationType>.backoff.max - Maximum backoff interval in ms.

* @param {number} notifConfig.queueProcessor.retry.<locationType>.backoff.jitter - Jitter factor.

* @param {number} notifConfig.queueProcessor.retry.<locationType>.backoff.factor - Exponential backoff multiplier.

* @param {Object} repConfig.queueProcessor.retry - Object keyed by location type

* (e.g., `aws_s3`, `azure`, `gcp`, `scality`), defining retry parameters for each location type.

* @param {number} repConfig.queueProcessor.retry.<locationType>.maxRetries - Maximum number of retry attempts.

* @param {number} repConfig.queueProcessor.retry.<locationType>.timeoutS - Timeout in seconds before giving up.

* @param {Object} repConfig.queueProcessor.retry.<locationType>.backoff - Exponential backoff configuration.

* @param {number} repConfig.queueProcessor.retry.<locationType>.backoff.min - Minimum backoff interval in ms.

* @param {number} repConfig.queueProcessor.retry.<locationType>.backoff.max - Maximum backoff interval in ms.

* @param {number} repConfig.queueProcessor.retry.<locationType>.backoff.jitter - Jitter factor.

* @param {number} repConfig.queueProcessor.retry.<locationType>.backoff.factor - Exponential backoff multiplier.

thanks, good catch. As you had guessed I copied/pasted it from the notification QueueProcessor.

bert-e · 2025-03-25T17:04:07Z

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

francoisferrand · 2025-03-26T07:06:15Z

extensions/replication/tasks/UpdateReplicationStatus.js

-            timeoutS:
-            rspState.repConfig.replicationStatusProcessor.retryTimeoutS,
-        });
+        super(rspState.repConfig.replicationStatusProcessor.retry);


retryParams are already on line 34, what is the point of this change?

(a cleanup may be good indeed, to pass the retryParams through the constructor instead of overriding them: but this is not what you are implementing here, and would change most backbeat tasks)

Suggested change

super(rspState.repConfig.replicationStatusProcessor.retry);

super();

Yes, that was my point about passing it in the constructor. I overlooked the unnecessary redundancy and will remove this.retryParams = this.repConfig.replicationStatusProcessor.retry;

Thanks.

francoisferrand · 2025-03-26T07:08:38Z

extensions/replication/utils/SetupReplication.js

-                retryTimeoutS, skipSourceBucketCreation, log } = params;
-        super({ timeoutS: retryTimeoutS });
+                repConfig, skipSourceBucketCreation, log } = params;
+        super(repConfig.queueProcessor.retry.scality);


replication may target other backends (not just scality), and each "kind" (scality, aws, azure...) has its own retry params:

seems weird to single-out the 'scality' retry params here

due to these target-specific params, are the (proper) retry params not already used deeper in the code, when doing the actual replication, i.e. is it really needed to set it here?

SetupReplication class fully configures CRR replication by performing all the housekeeping tasks needed for it to work (bucket existence/creation, versioning, IAM roles/policies, and the S3 replication configuration).

For internal replication (CRR), the type introduced is "scality".

francoisferrand · 2025-03-26T09:58:57Z

extensions/replication/tasks/ReplicateObject.js

-        super({
-            timeoutS: qpState.repConfig.queueProcessor.retryTimeoutS,
-        });
+        super();


should retryParams assignment be moved from line 43-53 to constructor param, then?

codecov · 2025-03-26T15:31:38Z

Codecov Report

Attention: Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.

Project coverage is 73.39%. Comparing base (4963f4b) to head (6d87c9f).
Report is 1 commits behind head on development/9.0.

Files with missing lines	Patch %	Lines
extensions/replication/utils/SetupReplication.js	0.00%	2 Missing ⚠️
extensions/replication/tasks/EchoBucket.js	0.00%	1 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
bin/replication.js	`0.00% <ø> (ø)`
...ions/notification/queueProcessor/QueueProcessor.js	`56.86% <ø> (ø)`
...sions/replication/queueProcessor/QueueProcessor.js	`72.96% <ø> (ø)`
extensions/replication/tasks/ReplicateObject.js	`91.49% <100.00%> (ø)`
...sions/replication/tasks/UpdateReplicationStatus.js	`78.37% <100.00%> (-0.12%)`	⬇️
extensions/replication/tasks/EchoBucket.js	`5.49% <0.00%> (ø)`
extensions/replication/utils/SetupReplication.js	`2.82% <0.00%> (ø)`

... and 5 files with indirect coverage changes

Components	Coverage Δ
Bucket Notification	`75.37% <ø> (ø)`
Core Library	`80.27% <ø> (+0.86%)`	⬆️
Ingestion	`70.14% <ø> (ø)`
Lifecycle	`77.08% <ø> (+0.20%)`	⬆️
Oplog Populator	`85.06% <ø> (ø)`
Replication	`58.59% <78.57%> (+0.02%)`	⬆️
Bucket Scanner	`85.60% <ø> (ø)`

@@                 Coverage Diff                 @@
##           development/9.0    #2633      +/-   ##
===================================================
+ Coverage            72.99%   73.39%   +0.40%     
===================================================
  Files                  201      201              
  Lines                13374    13373       -1     
===================================================
+ Hits                  9762     9815      +53     
+ Misses                3602     3548      -54     
  Partials                10       10

Flag	Coverage Δ
api:retry	`9.48% <0.00%> (+<0.01%)`	⬆️
api:routes	`9.29% <0.00%> (+<0.01%)`	⬆️
bucket-scanner	`85.60% <ø> (ø)`
ft_test:queuepopulator	`10.77% <0.00%> (+1.87%)`	⬆️
ingestion	`12.56% <0.00%> (+<0.01%)`	⬆️
lib	`7.35% <0.00%> (+0.01%)`	⬆️
lifecycle	`18.75% <0.00%> (+<0.01%)`	⬆️
notification	`1.06% <0.00%> (+<0.01%)`	⬆️
replication	`18.52% <78.57%> (-0.01%)`	⬇️
unit	`48.55% <71.42%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nicolas2bert · 2025-03-26T15:35:08Z

extensions/replication/tasks/ReplicateObject.js

-        if (this.destConfig && this.destConfig.bootstrapList) {
-            const destination = this.destConfig.bootstrapList
-                .find(endpoint => endpoint.site === this.site) || {};
-            this.destType = destination.type;


Removing this.destType since it does not seem to be used anywhere.

nicolas2bert · 2025-03-27T08:52:23Z

@bert-e approve

bert-e · 2025-03-27T08:52:34Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge bugfix/BB-673/retry into
development/9.0
w/9.1/bugfix/BB-673/retry will be merged into development/9.1

The following branches will NOT be impacted:

development/7.10
development/7.4
development/7.70
development/8.6

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

The following options are set: approve

bert-e · 2025-03-27T08:52:45Z

History mismatch

Merge commit #8118c88d5262b48c3d3dafb9bf434121801b0eb9 on the integration branch
w/9.1/bugfix/BB-673/retry is merging a branch which is neither the current
branch bugfix/BB-673/retry nor the development branch
development/9.1.

It is likely due to a rebase of the branch bugfix/BB-673/retry and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

The following options are set: approve

bert-e · 2025-03-27T08:53:47Z

History mismatch

Merge commit #722a5725b6cafcbbad1e641d5145f4abf8cb8708 on the integration branch
w/9.1/bugfix/BB-673/retry is merging a branch which is neither the current
branch bugfix/BB-673/retry nor the development branch
development/9.1.

It is likely due to a rebase of the branch bugfix/BB-673/retry and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

The following options are set: approve

nicolas2bert · 2025-03-27T08:53:53Z

@bert-e reset

bert-e · 2025-03-27T08:54:03Z

Reset complete

I have successfully deleted this pull request's integration branches.

The following options are set: approve

bert-e · 2025-03-27T08:54:13Z

Integration data created

I have created the integration data for the additional destination branches.

this pull request will merge bugfix/BB-673/retry into
development/9.0
w/9.1/bugfix/BB-673/retry will be merged into development/9.1

The following branches will NOT be impacted:

development/7.10
development/7.4
development/7.70
development/8.6

You can set option create_pull_requests if you need me to create
integration pull requests in addition to integration branches, with:

@bert-e create_pull_requests

The following options are set: approve

bert-e · 2025-03-27T08:54:26Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/9.0
✔️ development/9.1

The following branches have NOT changed:

development/7.10
development/7.4
development/7.70
development/8.6

Please check the status of the associated issue BB-673.

Goodbye nicolas2bert.

The following options are set: approve

Kerkesni reviewed Mar 25, 2025

View reviewed changes

Kerkesni approved these changes Mar 25, 2025

View reviewed changes

anurag4DSB approved these changes Mar 25, 2025

View reviewed changes

francoisferrand reviewed Mar 26, 2025

View reviewed changes

nicolas2bert force-pushed the bugfix/BB-673/retry branch from 26f5bae to 722a572 Compare March 26, 2025 15:29

nicolas2bert commented Mar 26, 2025

View reviewed changes

BB-673 Fix retry for replication and notification

6d87c9f

nicolas2bert force-pushed the bugfix/BB-673/retry branch from 722a572 to 6d87c9f Compare March 27, 2025 08:53

bert-e merged commit 6d87c9f into development/9.0 Mar 27, 2025
8 checks passed

bert-e deleted the bugfix/BB-673/retry branch March 27, 2025 08:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BB-673 Fix retry for replication and notification #2633

BB-673 Fix retry for replication and notification #2633

nicolas2bert commented Mar 25, 2025

bert-e commented Mar 25, 2025

bert-e commented Mar 25, 2025

Kerkesni Mar 25, 2025

nicolas2bert Mar 26, 2025

bert-e commented Mar 25, 2025

francoisferrand Mar 26, 2025

nicolas2bert Mar 26, 2025

francoisferrand Mar 26, 2025 •

edited

Loading

nicolas2bert Mar 26, 2025

francoisferrand Mar 26, 2025

codecov bot commented Mar 26, 2025 •

edited

Loading

nicolas2bert Mar 26, 2025

nicolas2bert commented Mar 27, 2025

bert-e commented Mar 27, 2025

bert-e commented Mar 27, 2025

bert-e commented Mar 27, 2025

nicolas2bert commented Mar 27, 2025

bert-e commented Mar 27, 2025

bert-e commented Mar 27, 2025

bert-e commented Mar 27, 2025

	super(rspState.repConfig.replicationStatusProcessor.retry);
	super();

BB-673 Fix retry for replication and notification #2633

BB-673 Fix retry for replication and notification #2633

Conversation

nicolas2bert commented Mar 25, 2025

bert-e commented Mar 25, 2025

Hello nicolas2bert,

bert-e commented Mar 25, 2025

Incorrect fix version

Kerkesni Mar 25, 2025

Choose a reason for hiding this comment

nicolas2bert Mar 26, 2025

Choose a reason for hiding this comment

bert-e commented Mar 25, 2025

Request integration branches

francoisferrand Mar 26, 2025

Choose a reason for hiding this comment

nicolas2bert Mar 26, 2025

Choose a reason for hiding this comment

francoisferrand Mar 26, 2025 • edited Loading

Choose a reason for hiding this comment

nicolas2bert Mar 26, 2025

Choose a reason for hiding this comment

francoisferrand Mar 26, 2025

Choose a reason for hiding this comment

codecov bot commented Mar 26, 2025 • edited Loading

Codecov Report

nicolas2bert Mar 26, 2025

Choose a reason for hiding this comment

nicolas2bert commented Mar 27, 2025

bert-e commented Mar 27, 2025

Integration data created

bert-e commented Mar 27, 2025

History mismatch

bert-e commented Mar 27, 2025

History mismatch

nicolas2bert commented Mar 27, 2025

bert-e commented Mar 27, 2025

Reset complete

bert-e commented Mar 27, 2025

Integration data created

bert-e commented Mar 27, 2025

francoisferrand Mar 26, 2025 •

edited

Loading

codecov bot commented Mar 26, 2025 •

edited

Loading