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.

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

Reducing list API payload #2627

Merged
merged 3 commits into from
Nov 7, 2024
Merged

Reducing list API payload #2627

merged 3 commits into from
Nov 7, 2024

Conversation

anushka567
Copy link
Member

@anushka567 anushka567 commented Oct 24, 2024

Description

Fetching only selected attributes using query.SetAttrSelection for ListObjects API which are sufficient for serving the request and reduce the payload for network call.
Reduction in payload : ~75%
Reduction in latency : ~8.5%
Also included : necessary changes in the fake bucket tests.

Perf Metrics for listing :

Test Directory Size ( in number of files) Mean Latency(Master) (in msec) Mean Latency(reduced payload) (in msec) Percentage Improvement
1000 61.153 56.325 7.89%
10000 462.775 410.701 11.25%
50000 2373.293 2103.235 11.38%
100000 4677.908 4354.92 6.90%
200000 9417.683 8628.266 8.38%

Note: More detailed metrics here

Link to the issue in case of a bug fix.

NA

Testing details

  1. Manual - NA
  2. Unit tests - NA
  3. Integration tests - NA

Copy link

codecov bot commented Oct 24, 2024

Codecov Report

Attention: Patch coverage is 94.73684% with 4 lines in your changes missing coverage. Please review.

Project coverage is 77.49%. Comparing base (ede87e1) to head (f25156b).
Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
internal/storage/bucket_handle.go 20.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2627      +/-   ##
==========================================
- Coverage   77.56%   77.49%   -0.07%     
==========================================
  Files         109      109              
  Lines       15576    15603      +27     
==========================================
+ Hits        12081    12092      +11     
- Misses       2979     2992      +13     
- Partials      516      519       +3     
Flag Coverage Δ
unittests 77.49% <94.73%> (-0.07%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@anushka567 anushka567 marked this pull request as ready for review October 24, 2024 13:45
@anushka567 anushka567 requested a review from a team as a code owner October 24, 2024 13:45
@anushka567 anushka567 requested a review from charith87 October 24, 2024 13:45
@kislaykishore kislaykishore requested review from a team, tritone and gargnitingoogle and removed request for a team and tritone October 24, 2024 13:45
@anushka567 anushka567 requested review from tritone and vadlakondaswetha and removed request for gargnitingoogle October 24, 2024 13:45
@anushka567 anushka567 added the execute-integration-tests Run only integration tests label Oct 25, 2024
internal/storage/bucket_handle.go Outdated Show resolved Hide resolved
internal/storage/fake/testing/bucket_tests.go Outdated Show resolved Hide resolved
internal/storage/bucket_handle.go Outdated Show resolved Hide resolved
@anushka567 anushka567 force-pushed the reduce-list-api-payload branch from 44603f3 to 98e6bb3 Compare October 25, 2024 09:27
@anushka567 anushka567 added the execute-perf-test Execute performance test in PR label Nov 4, 2024
internal/storage/fake/testing/bucket_tests.go Outdated Show resolved Hide resolved
internal/storage/bucket_handle.go Show resolved Hide resolved
internal/storage/bucket_handle.go Outdated Show resolved Hide resolved
@anushka567 anushka567 removed the execute-perf-test Execute performance test in PR label Nov 5, 2024
@anushka567 anushka567 merged commit 12de8b8 into master Nov 7, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
execute-integration-tests Run only integration tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants