search

near / nearcore

Reference client for NEAR Protocol
https://near.org
GNU General Public License v3.0
2.3k stars 600 forks source link

fix: Deprecate and cleanup ReceiptIdToShardId column #11691

Closed tayfunelmas closed 3 days ago

tayfunelmas commented 4 days ago

This addresses the issue in #11605. It allows to re-enable GC in testloop.

ReceiptIdToShardId column has not been updated and GC'ed consistently (which manifest itself as testloop failures when GC is enabled as indicated in #11605): 1) save_receipt_id_to_shard_id_for_block is not called in 2 paths in state sync (set_state_finalize) and catchup (block_catch_up_postprocess). This breaks the invariant that ReceiptIdToShardId is saved whenever OutgoingReceipts are saved. 2) GC does not check shard-tracking info and existence of the refcount when decrementing refcount of ReceiptIdToShardId if it exists in GC.

Since ReceiptIdToShardId is used to check certain conditions during resharding and not read in production, instead of fixing the situation (as in https://github.com/near/nearcore/pull/11668), we deprecate the column, remove the read and write operations to the column, and implement a new DB version that deleted the info in the column.

This PR replaces https://github.com/near/nearcore/pull/11668.

tayfunelmas commented 4 days ago

LGTM

Just for my understanding, was this column only used for testing resharding, but not in the actual resharding logic?

Yes, it is only used for testing the resharding logic, there is no read from the column in non-test code.

codecov[bot] commented 3 days ago

Codecov Report

Attention: Patch coverage is 21.42857% with 11 lines in your changes missing coverage. Please review.

Project coverage is 71.76%. Comparing base (8463ac2) to head (5bc21db).

Files Patch % Lines
core/store/src/migrations.rs 0.00% 8 Missing :warning:
chain/chain/src/garbage_collection.rs 0.00% 1 Missing :warning:
integration-tests/src/test_loop/builder.rs 0.00% 1 Missing :warning:
nearcore/src/migrations.rs 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #11691 +/- ## ========================================== - Coverage 71.80% 71.76% -0.04% ========================================== Files 790 790 Lines 161951 161879 -72 Branches 161951 161879 -72 ========================================== - Hits 116285 116173 -112 - Misses 40627 40672 +45 + Partials 5039 5034 -5 ``` | [Flag](https://app.codecov.io/gh/near/nearcore/pull/11691/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | Coverage Δ | | |---|---|---| | [backward-compatibility](https://app.codecov.io/gh/near/nearcore/pull/11691/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <0.00%> (+<0.01%)` | :arrow_up: | | [db-migration](https://app.codecov.io/gh/near/nearcore/pull/11691/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <0.00%> (+<0.01%)` | :arrow_up: | | [genesis-check](https://app.codecov.io/gh/near/nearcore/pull/11691/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.35% <0.00%> (+<0.01%)` | :arrow_up: | | [integration-tests](https://app.codecov.io/gh/near/nearcore/pull/11691/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `37.85% <14.28%> (-0.05%)` | :arrow_down: | | [linux](https://app.codecov.io/gh/near/nearcore/pull/11691/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `69.10% <21.42%> (-0.03%)` | :arrow_down: | | [linux-nightly](https://app.codecov.io/gh/near/nearcore/pull/11691/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.28% <21.42%> (-0.01%)` | :arrow_down: | | [macos](https://app.codecov.io/gh/near/nearcore/pull/11691/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `51.01% <15.38%> (-1.54%)` | :arrow_down: | | [pytests](https://app.codecov.io/gh/near/nearcore/pull/11691/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.58% <0.00%> (+<0.01%)` | :arrow_up: | | [sanity-checks](https://app.codecov.io/gh/near/nearcore/pull/11691/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.38% <0.00%> (+<0.01%)` | :arrow_up: | | [unittests](https://app.codecov.io/gh/near/nearcore/pull/11691/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `66.31% <15.38%> (-0.03%)` | :arrow_down: | | [upgradability](https://app.codecov.io/gh/near/nearcore/pull/11691/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.28% <0.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.