Skip to content

RUMM-2295, RUMM-2296, RUMM-2297: Improvement to the reading batch logic#992

Merged
0xnm merged 3 commits into
feature/sdkv2from
nogorodnikov/read_batch_improvements
Jul 22, 2022
Merged

RUMM-2295, RUMM-2296, RUMM-2297: Improvement to the reading batch logic#992
0xnm merged 3 commits into
feature/sdkv2from
nogorodnikov/read_batch_improvements

Conversation

@0xnm

@0xnm 0xnm commented Jul 21, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

What does this PR do?

This changes does the following:

  • RUMM-2297: Callback is added to notify if batch is not available. This allows to control the upload scheduling mechanism.
  • RUMM-2295: Removed the possibility of stack overflow in UploadWorker by converting recursive batch read to the usage of the queue with tasks.
  • RUMM-2296: Removed the bottleneck in UploadWorker: now if some feature has long upload which fails (and hence needs to retried), other features are going to have a chance to perform upload before retry happens.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@0xnm
0xnm requested a review from a team as a code owner July 21, 2022 10:02
@codecov-commenter

codecov-commenter commented Jul 21, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 86.48649% with 5 lines in your changes missing coverage. Please review.

Project coverage is 82.90%. Comparing base (da2dd64) to head (17cb5c7).

Files with missing lines Patch % Lines
.../android/core/internal/data/upload/UploadWorker.kt 88.89% 0 Missing and 3 partials ⚠️
...atadog/android/v2/core/internal/storage/Storage.kt 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@                Coverage Diff                @@
##           feature/sdkv2     #992      +/-   ##
=================================================
- Coverage          82.91%   82.90%   -0.01%     
=================================================
  Files                295      296       +1     
  Lines               9823     9841      +18     
  Branches            1600     1603       +3     
=================================================
+ Hits                8144     8158      +14     
  Misses              1195     1195              
- Partials             484      488       +4
Files with missing lines Coverage Δ
...v2/core/internal/data/upload/DataUploadRunnable.kt 100.00% <100.00%> (ø)
...id/v2/core/internal/storage/ConsentAwareStorage.kt 96.81% <100.00%> (+0.11%) ⬆️
...atadog/android/v2/core/internal/storage/Storage.kt 0.00% <0.00%> (ø)
.../android/core/internal/data/upload/UploadWorker.kt 85.71% <88.89%> (+2.38%) ⬆️

... and 6 files with indirect coverage changes

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

@xgouchet xgouchet added the size-medium This PR is medium sized label Jul 21, 2022
Base automatically changed from nogorodnikov/rumm-2347/provide-core-sdk-context to feature/sdkv2 July 21, 2022 10:28
mariusc83
mariusc83 approved these changes Jul 22, 2022
View reviewed changes

@mariusc83 mariusc83 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@0xnm
0xnm merged commit 8d75f58 into feature/sdkv2 Jul 22, 2022
@0xnm
0xnm deleted the nogorodnikov/read_batch_improvements branch July 22, 2022 08:26
@xgouchet xgouchet added this to the 1.16.0 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mariusc83 mariusc83 mariusc83 approved these changes
+1 more reviewer
@xgouchet xgouchet xgouchet approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size-medium This PR is medium sized

Projects

None yet

Milestone

1.16.0

Development

Successfully merging this pull request may close these issues.

4 participants

@0xnm @codecov-commenter @xgouchet @mariusc83