Locate object-storing shards #1635
No reviewers
Labels
No labels
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No milestone
No project
No assignees
6 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-node#1635
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "elebedeva/frostfs-node:feat/shard-info-for-object"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Added method to ControlService to find info about shards storing an object by object ID and added
locate
sub-command tofrostfs-cli control
.Signed-off-by: Ekaterina Lebedeva ekaterina.lebedeva@yadro.com
oid
f5c118742536744bbe9c
toa7ce8dc3ec
WIP: Locate object-storing shardto Locate object-storing shard@ -443,2 +443,4 @@
return s.hash
}
func (e *StorageEngine) GetShardByObjectID(ctx context.Context, obj oid.Address) ([]shard.Info, error) {
Need to take mtx here to iterate over shards.
Fixed.
a7ce8dc3ec
to70083f6e1c
70083f6e1c
to04a50e8c59
04a50e8c59
to02d25794fe
02d25794fe
to6829489621
6829489621
toecfe06bd7c
@ -0,0 +25,4 @@
var obj oid.ID
err = obj.DecodeString(req.GetBody().GetOid())
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
If request sets oid with an invalid format, then we can set
codes.InvalidArgument
instead ofcodes.Internal
. Thus, a client won't retryGetShardByObjectID
on invalid data :)The same for cid below
Thank you! Fixed.
@ -0,0 +67,4 @@
shardInfo := new(control.ShardInfo)
shardInfo.SetShard_ID(*info.ID)
shardInfo.SetMetabasePath(info.MetaBaseInfo.Path)
shardInfo.Blobstor = blobstorInfoToProto(info.BlobStorInfo)
Why don't we use
SetBlobstor
and use explicit assignment?Changed to
shardInfo.SetBlobstor()
.@ -0,0 +24,4 @@
"github.com/spf13/cobra"
)
var objectLocateCmd = &cobra.Command{
control
service commands should be called withfrostfs-cli control ...
Otherwise, it creates confusion
Moved this command to
frostfs-cli control
. I thought since this command finds object's location - it belongs tofrostfs-cli object
.@ -0,0 +114,4 @@
}
}
func prettyPrintShardsJSON(cmd *cobra.Command, ii []control.ShardInfo) {
All this info is unrelated to the command.
What is the purpose?
I looked at
shards list
and it has--json
flag, so I thought json-formatted output here might also be useful.@ -445,0 +445,4 @@
func (e *StorageEngine) GetShardByObjectID(ctx context.Context, obj oid.Address) ([]shard.Info, error) {
var err error
info := make([]shard.Info, 0, len(e.shards))
We do not actually know slice capacity, and most likely there is 0 or 1 shard.
Preallocation is not needed here, just
var info []shard.Info
.Fixed
@ -0,0 +16,4 @@
)
func (s *Server) GetShardByObjectID(ctx context.Context, req *control.GetShardByObjectIDRequest) (*control.GetShardByObjectIDResponse, error) {
// verify request
Please, remove this comment, I find it useless.
Fixed
@ -90,2 +90,4 @@
// StartShardRebuild starts shard rebuild process.
rpc StartShardRebuild(StartShardRebuildRequest) returns (StartShardRebuildResponse);
// GetShardByObjectID returns shard info where object is stored.
I think
GetShardByObjectID
is a bad naming --ObjectID
is not some persistent identifier of a shard, it is something that a shard can store.May be sth like
ListShardsForObject
?Thanks, renamed
GetShardByObjectID
toListShardsForObject
.@ -732,0 +735,4 @@
message GetShardByObjectIDRequest {
message Body {
string oid = 1;
object_id
andcontainer_id
would be better (in accordance with other fields likechain_id
orshard_id
)Fixed
ecfe06bd7c
to51ffe64227
@ -0,0 +70,4 @@
}
}
func readObjectAddress(cmd *cobra.Command, cnr *cid.ID, obj *oid.ID) oid.Address {
duplicate
func readObjectAddress(cmd *cobra.Command, cnr *cid.ID, obj *oid.ID) oid.Address {
Made
object.readObjectAddress()
public and reused it inlocateObject()
.@ -445,0 +450,4 @@
Address: obj,
}
e.iterateOverUnsortedShards(func(hs hashedShard) (stop bool) {
Engine.Exists
has some error handling:func (e *StorageEngine) exists(ctx context.Context, shPrm shard.ExistsPrm) (bool, bool, error) {
Is this error checking not performed here intentionally?
It was intentional as I assumed we should stop on every error. I didn't consider that
ObjectNotFound
could be an error.Added error checks.
@ -0,0 +21,4 @@
}
func initControlLocateObjectCmd() {
commonflags.Init(locateObjectCmd)
Control commands have different common flags, see the command for shard listing
initControlFlags(listShardsCmd)
Please double-check and fix if needed
Fixed
@ -0,0 +49,4 @@
req.SetBody(body)
signRequest(cmd, pk, req)
cli := internalclient.GetSDKClientByFlag(cmd, pk, commonflags.RPC)
You should use control endpoint, please see the command for shard listing
cli := getClient(cmd, pk)
Fixed
@ -732,0 +735,4 @@
message ListShardsForObjectRequest {
message Body {
string object_id = 1;
How about using bytes instead of string? An ID as bytes takes 32 bytes, the same ID as a string takes about 44 bytes. Also I see we already use bytes in control IR service.
What do you think?
I think it is an an error that we use
bytes
for shardID. String is more human readable and control API is not so performance critical to save 12 bytes:)51ffe64227
toa6bf8437d1
Locate object-storing shardto WIP: Locate object-storing sharda6bf8437d1
toeaf7688ee9
eaf7688ee9
to4a54367233
WIP: Locate object-storing shardto Locate object-storing shardLocate object-storing shardto Locate object-storing shards@ -445,0 +461,4 @@
return true
}
var siErr *objectSDK.SplitInfoError
ECInfoError
tooAdded check for
ECInfoError
4a54367233
to466779e627
I find this very useful 👍
@ -445,0 +453,4 @@
Address: obj,
}
e.iterateOverUnsortedShards(func(hs hashedShard) (stop bool) {
Let's iterate over sorted shards
Why? It is required to find all shards store object, but not the first shard stores object.
Ok, nevermind
@ -445,0 +461,4 @@
return true
}
var siErr *objectSDK.SplitInfoError
Move these variables definitions outside shard iterating
@ -445,0 +462,4 @@
}
var siErr *objectSDK.SplitInfoError
if errors.As(exErr, &siErr) {
Let's merge similar check together and make less
if
s@ -445,0 +482,4 @@
e.reportShardError(ctx, hs, "could not check existence of object in shard", exErr, zap.Stringer("address", prm.Address))
}
}
if res.Exists() {
I think
Exists
should not be called if an error was returned. Now this happens if you get an unexpected error you then report. You can either add returnfalse
after error reporting or useelse
or do something elseView command line instructions
Checkout
From your project repository, check out a new branch and test the changes.