Conversation
End-rey
requested review from
carpawell,
cthulhu-rider and
roman-khimov
as code owners
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3889 +/- ##
==========================================
- Coverage 26.78% 26.77% -0.02%
==========================================
Files 677 677
Lines 44616 44665 +49
==========================================
+ Hits 11952 11957 +5
- Misses 31553 31593 +40
- Partials 1111 1115 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
End-rey
force-pushed
the
storage-shard-id-generation
branch
3 times, most recently
from
ec034b9 to
2773eac
Compare
| } | ||
|
|
||
| // DecodeString decodes base58 string into ID. | ||
| func DecodeString(s string) (ID, error) { |
There was a problem hiding this comment.
DecodeIDString, this package has other things besides ID, so common.DecodeString can mean anything.
| } | ||
|
|
||
| // NewFromBytes constructs ID from fixed-size raw bytes. | ||
| func NewFromBytes(v []byte) (ID, error) { |
| s := cons(t) | ||
| require.NoError(t, s.Open(false)) | ||
| require.NoError(t, s.Init()) | ||
| require.NoError(t, s.Init(common.ID{})) |
There was a problem hiding this comment.
I'm looking at how many changes there are like this and maybe some InitIdentified(), InitAsSecondary() or InitWithID() would be helpful. Most of the time ID is zero. This however will affect blobstor interface. Opinions are welcome, current code works too, just the necessity of passing this common.ID{} around that bugs me.
There was a problem hiding this comment.
I would make this change only for blobstor, since it’s the only one whose code might encounter a situation where the ID doesn’t need to be specified - it can retrieve or generate it on its own. For metabase and writecache, the current API is only used with IDs, and they’re empty only in tests because it’s not that important there.
End-rey
force-pushed
the
storage-shard-id-generation
branch
from
2773eac to
5aa2ad9
Compare
Extract shard ID into `pkg/local_object_storage/blobstor/common` and switch internal usages to `common.ID`. This moves the type closer to its storage domain and avoids base58-oriented handling in internal storage. Signed-off-by: Andrey Butusov <andrey@nspcc.io>
End-rey
force-pushed
the
storage-shard-id-generation
branch
from
5aa2ad9 to
4c4a082
Compare
Make fstree/blobstor the canonical shard ID source and propagate the resulting ID to metabase and write-cache during shard initialization. Align shard and engine initialization with this flow. Store fstree subtype in the descriptor to distinguish blobstor from write-cache in recovery scenarios, with blobstor as the default subtype. Closes #3859. Signed-off-by: Andrey Butusov <andrey@nspcc.io>
2 participants
Closes #3859.
Two design points changed in this PR and I would like extra attention there:
shard.New()now callsResolveID()eagerly, so shard ID is known beforeInit().fstree/blobstornow works throughInit(shardID), and the same ID is then propagated tometabaseandwrite-cache.This is consistent with the earlier discussion, but these changes move some responsibility across
New/Initboundaries, so I would like to double-check that the lifecycle still looks right.