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

Firestore: QueryTest.java: Use a WriteBatch to delete documents rather than a transaction #5167

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

dconeybe
Copy link
Contributor

This is a port of firebase/firebase-js-sdk#7415

@github-actions
Copy link
Contributor

github-actions bot commented Jul 14, 2023

📝 PRs merging into main branch

Our main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released.

@google-oss-bot
Copy link
Contributor

Coverage Report 1

Affected Products

  • firebase-firestore

    Overall coverage changed from 44.35% (c4b5a92) to 44.33% (c5b7e7d) by -0.02%.

    FilenameBase (c4b5a92)Merge (c5b7e7d)Diff
    DeleteMutation.java95.24%90.48%-4.76%
    LruGarbageCollector.java97.27%93.64%-3.64%
    PatchMutation.java100.00%98.39%-1.61%

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/zVPm17nGDj.html

@github-actions
Copy link
Contributor

Unit Test Results

   162 files  ±0     162 suites  ±0   2m 22s ⏱️ +17s
1 164 tests ±0  1 148 ✔️ ±0  16 💤 ±0  0 ±0 
2 328 runs  ±0  2 296 ✔️ ±0  32 💤 ±0  0 ±0 

Results for commit 0cb2244. ± Comparison against base commit c4b5a92.

@google-oss-bot
Copy link
Contributor

@google-oss-bot
Copy link
Contributor

Startup Time Report 1

Note: Layout is sometimes suboptimal due to limited formatting support on GitHub. Please check this report on GCS.

Notes

Startup Times

  • fire-fst

    DeviceStatisticsDistributions
    oriole-32
    Percentilec4b5a92c5b7e7dDiffSignificant (?)
    p10329 ±17 μs314 ±15 μs-15.2 μs (-4.6%)NO
    p25345 ±18 μs328 ±22 μs-16.5 μs (-4.8%)NO
    p50382 ±52 μs365 ±53 μs-17.7 μs (-4.6%)NO
    p75437 ±88 μs440 ±102 μs+3.10 μs (+0.7%)NO
    p90566 ±227 μs556 ±198 μs-9.82 μs (-1.7%)NO

    20 test runs in comparison
    CommitTest Runs
    c4b5a92
    • 2023-07-14_18:27:06.618577_cnjR
    • 2023-07-14_18:27:06.621619_YdWZ
    • 2023-07-14_18:27:06.621632_hoiX
    • 2023-07-14_18:27:06.621640_dONz
    • 2023-07-14_18:27:06.621990_bMbC
    • 2023-07-14_18:27:06.621998_Tgxt
    • 2023-07-14_18:27:06.622004_SYou
    • 2023-07-14_18:27:06.622009_Xkrv
    • 2023-07-14_18:27:06.622015_VIkF
    • 2023-07-14_18:27:06.622021_TgGe
    c5b7e7d
    • 2023-07-14_20:35:27.597377_dAMK
    • 2023-07-14_20:35:27.600536_cFZK
    • 2023-07-14_20:35:27.600548_FmqD
    • 2023-07-14_20:35:27.600555_tUew
    • 2023-07-14_20:35:27.600561_lxOh
    • 2023-07-14_20:35:27.600568_Biex
    • 2023-07-14_20:35:27.600578_BVqJ
    • 2023-07-14_20:35:27.600584_rWvs
    • 2023-07-14_20:35:27.600589_rMlU
    • 2023-07-14_20:35:27.600594_wPPg
    redfin-30
    Percentilec4b5a92c5b7e7dDiffSignificant (?)
    p10611 ±31 μs611 ±29 μs+50.4 ns (+0.0%)NO
    p25626 ±38 μs635 ±34 μs+9.04 μs (+1.4%)NO
    p50647 ±45 μs660 ±41 μs+12.2 μs (+1.9%)NO
    p75678 ±54 μs699 ±50 μs+20.5 μs (+3.0%)NO
    p90719 ±67 μs799 ±150 μs+80.1 μs (+11.1%)NO

    20 test runs in comparison
    CommitTest Runs
    c4b5a92
    • 2023-07-14_18:27:06.618577_cnjR
    • 2023-07-14_18:27:06.621619_YdWZ
    • 2023-07-14_18:27:06.621632_hoiX
    • 2023-07-14_18:27:06.621640_dONz
    • 2023-07-14_18:27:06.621990_bMbC
    • 2023-07-14_18:27:06.621998_Tgxt
    • 2023-07-14_18:27:06.622004_SYou
    • 2023-07-14_18:27:06.622009_Xkrv
    • 2023-07-14_18:27:06.622015_VIkF
    • 2023-07-14_18:27:06.622021_TgGe
    c5b7e7d
    • 2023-07-14_20:35:27.597377_dAMK
    • 2023-07-14_20:35:27.600536_cFZK
    • 2023-07-14_20:35:27.600548_FmqD
    • 2023-07-14_20:35:27.600555_tUew
    • 2023-07-14_20:35:27.600561_lxOh
    • 2023-07-14_20:35:27.600568_Biex
    • 2023-07-14_20:35:27.600578_BVqJ
    • 2023-07-14_20:35:27.600584_rWvs
    • 2023-07-14_20:35:27.600589_rMlU
    • 2023-07-14_20:35:27.600594_wPPg
  • timeToInitialDisplay

    DeviceStatisticsDistributions
    oriole-32
    Percentilec4b5a92c5b7e7dDiffSignificant (?)
    p10201 ±8 ms204 ±2 ms+2.44 ms (+1.2%)NO
    p25207 ±8 ms210 ±2 ms+3.09 ms (+1.5%)NO
    p50215 ±8 ms218 ±2 ms+3.37 ms (+1.6%)NO
    p75223 ±9 ms227 ±3 ms+3.77 ms (+1.7%)NO
    p90232 ±8 ms239 ±4 ms+6.91 ms (+3.0%)NO

    20 test runs in comparison
    CommitTest Runs
    c4b5a92
    • 2023-07-14_18:27:06.618577_cnjR
    • 2023-07-14_18:27:06.621619_YdWZ
    • 2023-07-14_18:27:06.621632_hoiX
    • 2023-07-14_18:27:06.621640_dONz
    • 2023-07-14_18:27:06.621990_bMbC
    • 2023-07-14_18:27:06.621998_Tgxt
    • 2023-07-14_18:27:06.622004_SYou
    • 2023-07-14_18:27:06.622009_Xkrv
    • 2023-07-14_18:27:06.622015_VIkF
    • 2023-07-14_18:27:06.622021_TgGe
    c5b7e7d
    • 2023-07-14_20:35:27.597377_dAMK
    • 2023-07-14_20:35:27.600536_cFZK
    • 2023-07-14_20:35:27.600548_FmqD
    • 2023-07-14_20:35:27.600555_tUew
    • 2023-07-14_20:35:27.600561_lxOh
    • 2023-07-14_20:35:27.600568_Biex
    • 2023-07-14_20:35:27.600578_BVqJ
    • 2023-07-14_20:35:27.600584_rWvs
    • 2023-07-14_20:35:27.600589_rMlU
    • 2023-07-14_20:35:27.600594_wPPg
    redfin-30
    Percentilec4b5a92c5b7e7dDiffSignificant (?)
    p10247 ±5 ms267 ±3 ms+19.9 ms (+8.1%)MAYBE
    p25253 ±5 ms273 ±3 ms+19.7 ms (+7.8%)MAYBE
    p50261 ±5 ms281 ±4 ms+20.7 ms (+8.0%)MAYBE
    p75269 ±6 ms290 ±5 ms+20.4 ms (+7.6%)MAYBE
    p90279 ±7 ms303 ±8 ms+24.7 ms (+8.9%)MAYBE

    20 test runs in comparison
    CommitTest Runs
    c4b5a92
    • 2023-07-14_18:27:06.618577_cnjR
    • 2023-07-14_18:27:06.621619_YdWZ
    • 2023-07-14_18:27:06.621632_hoiX
    • 2023-07-14_18:27:06.621640_dONz
    • 2023-07-14_18:27:06.621990_bMbC
    • 2023-07-14_18:27:06.621998_Tgxt
    • 2023-07-14_18:27:06.622004_SYou
    • 2023-07-14_18:27:06.622009_Xkrv
    • 2023-07-14_18:27:06.622015_VIkF
    • 2023-07-14_18:27:06.622021_TgGe
    c5b7e7d
    • 2023-07-14_20:35:27.597377_dAMK
    • 2023-07-14_20:35:27.600536_cFZK
    • 2023-07-14_20:35:27.600548_FmqD
    • 2023-07-14_20:35:27.600555_tUew
    • 2023-07-14_20:35:27.600561_lxOh
    • 2023-07-14_20:35:27.600568_Biex
    • 2023-07-14_20:35:27.600578_BVqJ
    • 2023-07-14_20:35:27.600584_rWvs
    • 2023-07-14_20:35:27.600589_rMlU
    • 2023-07-14_20:35:27.600594_wPPg

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/Y9AinSBTC4/index.html

@dconeybe dconeybe requested a review from milaGGL July 15, 2023 02:35
Copy link
Contributor

@milaGGL milaGGL left a comment

Choose a reason for hiding this comment

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

LGTM

@dconeybe dconeybe merged commit 9471221 into master Jul 17, 2023
@dconeybe dconeybe deleted the dconeybe/BloomFilterTestUseWriteBatchToDelete branch July 17, 2023 17:55
dconeybe added a commit that referenced this pull request Jul 20, 2023
… of a WriteBatch to avoid affecting the local cache.

This fixes the bug introduced by #5167 where WriteBatch affects the local cache (which I didn't realize).
@firebase firebase locked and limited conversation to collaborators Aug 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants