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

Fix partition location issue and refactoring #1977

Merged
merged 33 commits into from
Jan 22, 2023
Merged

Conversation

peterlimg
Copy link
Member

Fixes

  • Fix partition location overflow after deleting. The last item in the last partition's location will be moved to prior parititions as a replacement, however the item's user does not know, and would try to get it with old location, it would fail either with error of item not found or location overflow (beyond the partitions num)
  • Fix unit64 overflow caused by addBlobber on updating allocation

Changes

  • Removed the outside reference of partition locations, all will be managed internally in the partition package itself.
  • Adjust the code to remove the using of location in blobber allocations
  • Refactor the partitions packge to remove redundant 'randomSelector' and unused Callback methods.
  • Change to bind allocation to blobbers when create new allocation, and unbind only when blobber is removed from the allocation or the allocation is finalized or canceled
  • Adjust related challenge ready partition
  • Write unit tests to cover the changes for both partititons package and storagesc

Need to be mentioned in CHANGELOG.md?

Tests

Tasks to complete before merging PR:

  • Ensure system tests are passing. If not Run them manually to check for any regressions 📋
  • Do any new system tests need added to test this change? do any existing system tests need updated? If so create a PR at 0chain/system_test
  • Merge your system tests PR to master AFTER merging this PR

Associated PRs (Link as appropriate):

  • blobber:
  • gosdk:
  • system_test:
  • zboxcli:
  • zwalletcli:
  • Other: ...

It's not the challenge generation SC's responsibility to do this job, if it does, it should remove itself from all blobbers instead of just the selected one.
Rather than bind them on committing_connection SC, and unbind them when there's no data stored in the allocation, we connect them immediately when creating the allocation, and only unbind them when removing blobber from the allocation, or finalize/cancel the allocation. In this way, we can avoid the potential inconsistent state and avoid unnecessary add/remove operations.
## When new allocation
- assert blobber allocation added
- assert blobber total offer is initialized

## When update allocation
- assert add blobber will add new blobber allocation
- assert remove blobber will remove the blobber allocation
- assert remove blobber and it's the last bound allocation, the blobber will be removed from challenge ready partition

## When receive commit connection SC
- assert blobber is added to challenge ready partition

## When finalize allocation
- assert the allocation is removed from all bound blobbers
- assert blobber is removed from challenge ready if no allocation is bound to it
* staging: (134 commits)
  added docs
  added docs
  added docs
  Add authorizers total rewards
  Add validator's total rewards
  Add blobber total rewards
  Add sharders total rewards
  Add miners total rewards
  added docs
  fixed ambiguous field error
  returned struct{}
  skipped tests
  worked out comments
  worked out comments
  worked out comments
  Add default value to bucket_id
  Add bucket_id to upsert
  worked out comments
  updated dependency versions
  updated dependency versions upgraded lru cache
  ...
@codecov-commenter
Copy link

codecov-commenter commented Jan 12, 2023

Codecov Report

Merging #1977 (e0ce231) into staging (f8be3bc) will increase coverage by 0.15%.
The diff coverage is 51.39%.

@@             Coverage Diff             @@
##           staging    #1977      +/-   ##
===========================================
+ Coverage    26.26%   26.42%   +0.15%     
===========================================
  Files          370      370              
  Lines        61353    61252     -101     
===========================================
+ Hits         16117    16186      +69     
+ Misses       43322    43142     -180     
- Partials      1914     1924      +10     
Flag Coverage Δ
Unit-Tests 26.42% <51.39%> (+0.15%) ⬆️

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

Impacted Files Coverage Δ
...ain.net/smartcontract/storagesc/benchmark_setup.go 4.74% <0.00%> (+0.05%) ⬆️
...e/go/0chain.net/smartcontract/storagesc/blobber.go 43.83% <17.64%> (-0.32%) ⬇️
...o/0chain.net/smartcontract/storagesc/allocation.go 50.69% <20.00%> (+1.39%) ⬆️
...de/go/0chain.net/smartcontract/storagesc/models.go 55.75% <25.00%> (-2.79%) ⬇️
code/go/0chain.net/smartcontract/minersc/fees.go 42.11% <33.33%> (ø)
...hain.net/smartcontract/partitions/partition_gen.go 39.34% <37.03%> (+14.34%) ⬆️
...artcontract/storagesc/challenge_ready_partition.go 58.33% <47.36%> (+1.19%) ⬆️
...o/0chain.net/smartcontract/partitions/partition.go 62.65% <50.00%> (+4.22%) ⬆️
...smartcontract/storagesc/blobber_alloc_partition.go 47.05% <50.00%> (+4.95%) ⬆️
...o/0chain.net/smartcontract/storagesc/models_gen.go 34.19% <50.00%> (-0.98%) ⬇️
... and 21 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@dabasov dabasov left a comment

Choose a reason for hiding this comment

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

lgtm

# Conflicts:
#	code/go/0chain.net/smartcontract/storagesc/challenge_test.go
@peterlimg peterlimg merged commit 31c960f into staging Jan 22, 2023
@peterlimg peterlimg deleted the fix/partitions-loc branch January 22, 2023 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants