Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bugfix/s3 utils 188 support metrics above max safe #334

Merged
merged 17 commits into from
Jan 28, 2025
Merged

Bugfix/s3 utils 188 support metrics above max safe #334

merged 17 commits into from
Jan 28, 2025

Conversation

benzekrimaha
Copy link
Contributor

No description provided.

@bert-e
Copy link
Contributor

bert-e commented Jan 17, 2025

Hello benzekrimaha,

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
Copy link
Contributor

bert-e commented Jan 17, 2025

Incorrect fix version

The Fix Version/s in issue S3UTILS-188 contains:

  • 1.14.17

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

  • 1.14.18

  • 1.15.8

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

@benzekrimaha benzekrimaha force-pushed the bugfix/S3UTILS-188-support-metrics-above-max-safe branch 2 times, most recently from 56798e7 to 9325b3f Compare January 23, 2025 08:25
@benzekrimaha
Copy link
Contributor Author

/approve

@williamlardier
Update eslint config to support modern bigint notations
2dfe551 
Issue: S3UTILS-188
@williamlardier
Update and correct dev dependencies
d48a14f 
- Jest version was too old to support BigInts
- randomatic was used in some tests, yet never part of the project

Issue: S3UTILS-188
@williamlardier
Bump Arsenal
2004fd7 
Issue: S3UTILS-188
@williamlardier
[maintenance] keep CI up to date
d5d2b0a 
Issue: S3UTILS-188
@williamlardier
Update bucketVersionsStats to work with BigInts
b877a86 
Issue: S3UTILS-188
@williamlardier
Update VeeamSOSAPI consumption cronjob to support bigint
4b3da65 
- We are backward compatible
- But we now store the values as Long in MongoDB, explicitly,
  as it would be the case with large values anyway.

Issue: S3UTILS-188
@williamlardier
Update consolidate metrics to accept number and bigints
e382490 
- Output always bigint
- The conversion in place is to ensure backward compatibility:
  values might be numbers after an upgrade. It can be safely
  removed in an upcoming version once we confirm all customers
  are up to date.

Issue: S3UTILS-188
@williamlardier
Handle bigint communications internally
b447ecd 
- BigInts are serialized with a __bigint field and passed as
  string.
- When the receiver gets the data, the value is automatically
  converted into.

Issue: S3UTILS-188
@williamlardier williamlardier force-pushed the bugfix/S3UTILS-188-support-metrics-above-max-safe branch 2 times, most recently from bab3b6d to da1658f Compare January 24, 2025 12:50
@williamlardier
Copy link
Contributor

The PR was updated to :

  • Compute all metrics with bigints
  • Store the storage metrics as Long in MongoDB
  • Store the global metrics (in the document with _id countitems) as numbers, to not change anything. The values here will be imprecise but much more work is required if we want to support it properly everywhere.

williamlardier
williamlardier reviewed Jan 24, 2025
View reviewed changes
package.json Outdated Show resolved Hide resolved
@williamlardier williamlardier marked this pull request as ready for review January 24, 2025 14:53
@williamlardier williamlardier force-pushed the bugfix/S3UTILS-188-support-metrics-above-max-safe branch from da1658f to a8b1892 Compare January 24, 2025 15:37
@williamlardier
Copy link
Contributor

PR updated with a unit test to ensure we compute the values properly when we have bigints. Too hard to test as functional test: we won't be uploading large objects in the CI.

@williamlardier williamlardier force-pushed the bugfix/S3UTILS-188-support-metrics-above-max-safe branch 3 times, most recently from e3cc316 to d396d9c Compare January 25, 2025 01:15
@williamlardier williamlardier force-pushed the bugfix/S3UTILS-188-support-metrics-above-max-safe branch from c6ba83c to c034938 Compare January 27, 2025 08:12
@williamlardier williamlardier force-pushed the bugfix/S3UTILS-188-support-metrics-above-max-safe branch 2 times, most recently from a2ee69e to 07455d2 Compare January 27, 2025 13:46
@williamlardier williamlardier force-pushed the bugfix/S3UTILS-188-support-metrics-above-max-safe branch from 07455d2 to 866704f Compare January 27, 2025 14:43
@williamlardier williamlardier force-pushed the bugfix/S3UTILS-188-support-metrics-above-max-safe branch from 866704f to 4ad81fa Compare January 27, 2025 14:45
@williamlardier williamlardier force-pushed the bugfix/S3UTILS-188-support-metrics-above-max-safe branch from 4ad81fa to e79a2f6 Compare January 28, 2025 11:07
@scality scality deleted a comment from benzekrimaha Jan 28, 2025
@williamlardier
Copy link
Contributor

ping

@scality scality deleted a comment from bert-e Jan 28, 2025
@scality scality deleted a comment from bert-e Jan 28, 2025
@scality scality deleted a comment from bert-e Jan 28, 2025
@bert-e
Copy link
Contributor

bert-e commented Jan 28, 2025

Build failed

The build for commit did not succeed in branch w/1.15/bugfix/S3UTILS-188-support-metrics-above-max-safe

The following options are set: approve

@bert-e
Copy link
Contributor

bert-e commented Jan 28, 2025

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

  • ✔️ development/1.14

  • ✔️ development/1.15

The following branches have NOT changed:

  • development/1.13
  • development/1.4

Please check the status of the associated issue S3UTILS-188.

Goodbye benzekrimaha.

The following options are set: approve

@bert-e bert-e merged commit e79a2f6 into development/1.14 Jan 28, 2025
10 checks passed
@bert-e bert-e deleted the bugfix/S3UTILS-188-support-metrics-above-max-safe branch January 28, 2025 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@williamlardier williamlardier williamlardier approved these changes

@francoisferrand francoisferrand francoisferrand approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@benzekrimaha @bert-e @williamlardier @francoisferrand