[feature] Add bulk_operations invalidation limit

[donaldong]

Summary

This adds a limit to the number of records we would select from the database. Previously I thought we're selecting by uniq_by columns -- I was confused and wrong. We're actually selecting by the columns_to_update so the number of records can be super big.

This PR:

• Sets such limit. If we're going to load many records; simply invalidate everything
• We only care about the columns_to_update if they overlap with the columns we're memorizing
• Adds monitoring

Test Plan

• ci

[donaldong]

update

[codecov-io]

Codecov Report

Merging #31 (df61e80) into base/donaldong/feature_add_bulk_o76d8ee5 (2b8173f) will decrease coverage by 0.08%.
The diff coverage is 94.87%.

@@                             Coverage Diff                              @@
##           base/donaldong/feature_add_bulk_o76d8ee5      #31      +/-   ##
============================================================================
- Coverage                                     97.05%   96.97%   -0.09%     
============================================================================
  Files                                            32       32              
  Lines                                          1901     1916      +15     
============================================================================
+ Hits                                           1845     1858      +13     
- Misses                                           56       58       +2     

Impacted Files	Coverage Δ
lib/redis_memo/memoize_query/invalidation.rb	93.82% <93.54%> (-1.77%)	⬇️
lib/redis_memo/memoize_query.rb	98.52% <100.00%> (+0.02%)	⬆️
lib/redis_memo/options.rb	85.71% <100.00%> (+0.29%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b8173f...df61e80. Read the comment docs.

[donaldong]

add a test case

[katyho]

I wonder if it'd be better to just look at the total # of records being imported, to avoid this extra query / extra computation to iterate through all the records.

[donaldong]

extra computation to iterate through all the records.

Well this will send a SELECT count(*) query.

By sending this query we can avoid the expensive payload transfer and activerecord record installation.

I wonder if it'd be better to just look at the total # of records being imported

I don't think this would do. For example, if we on_duplicate_key_update: [:visibility], and we are also memorizing visibility, we could still be getting a lot of records back from the database

[donaldong]

Actually, now that I think about it, we can still query by uniq_by if we're really querying by uniq_by instead of those columns to update! That should be more efficient and we don't need to worry about the size limit since the uniq_by set is smaller or equal to the record set we're importing.

[donaldong]

nvm -- we cannot really query by uniq_by only because the uniq_by value might or might not exist -- it could be something filled by the database. So yeah I think query by the columns_to_update is still the best we can do.

[donaldong]

the best we can do

not really the best we can do, but I think it's good enough for now. Let's save more optimizations for later

[katyho]

Can we add unit tests / modify existing ones to check the logic we originally missed here?

E.g. if we're importing Site.import(records, on_duplicate_key_update: [:a, :b]), but we only memoize the column :a, we shouldn't be querying / invalidating :b

[donaldong]

Good idea! Will add this test case.

[donaldong]

modify test case