search

eBay / HomeObject

Replicated BLOB Store built upon HomeStore
Apache License 2.0
6 stars 15 forks source link

correct PG/Shard metablk client name during recovery #112

Closed zichanglai closed 1 year ago

zichanglai commented 1 year ago

This PR depends on @JacksonYao287 's latest PR to fix the HO build failure: https://github.com/eBay/HomeObject/pull/111

codecov-commenter commented 1 year ago

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (a608582) 78.90% compared to head (ea42df1) 78.74%. Report is 8 commits behind head on main.

Files Patch % Lines
src/lib/homestore_backend/hs_blob_manager.cpp 81.81% 2 Missing :warning:
src/lib/homestore_backend/hs_homeobject.cpp 88.88% 1 Missing and 1 partial :warning:
src/lib/memory_backend/mem_blob_manager.cpp 75.00% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #112 +/- ## ========================================== - Coverage 78.90% 78.74% -0.16% ========================================== Files 28 29 +1 Lines 1000 1035 +35 Branches 97 101 +4 ========================================== + Hits 789 815 +26 - Misses 150 160 +10 + Partials 61 60 -1 ```

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

zichanglai commented 1 year ago

lgtm

Can we disable the default constructor of homestore::superblk<T> ? This seems risky.

actually, when I take a look on the implementation of HS superblock, it will not make any mistakes even we have not correct the name of the metablk because sb.write() function will not use the metablk name again when sb has a cache metablk handler in here: https://github.com/eBay/HomeStore/blob/6897d3eca84506c0cbb36ec77fd38da03dc6aa80/src/include/homestore/superblk_handler.hpp#L76. The name is used only when the sb is first time to write. when we recover, it will use the cache metablk as the destination for the next write. but it will make coder more perfect if we had set it correctly.

yamingk commented 1 year ago

@zichanglai I forget to mention there is another sb in HomeObject svc_info_superblk_t that is also using the similar logic during recovery, can you also check whether it needs to be fixied?

zichanglai commented 1 year ago

similar

Hi @yamingk , I had check it before, svc_info_superblock do not have this issue as it construct with correct client name: https://github.com/eBay/HomeObject/blob/a85345a29ef2dc0395503bc5db1e990cc1e4f430/src/lib/homestore_backend/hs_homeobject.cpp#L120C53-L120C53