TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 100 Pull requests 4 Projects Releases 1 Activity Actions

node: Implement Get\Head requests for EC object #1103

Merged
fyrchik merged 1 commit from acid-ant/frostfs-node:feature/ec-get-head into master 2024-09-04 19:51:08 +00:00
Conversation 0 Commits 1 Files changed 30 +575 -7
acid-ant commented 2024-04-22 10:32:19 +00:00
Member
Copy link

With these changes it is possible to get\head erasure-coded objects via cli.
Example output for GET:

$ frostfs-cli -r s01.frostfs.devenv:8080 -c cfg.yml container create --policy 'EC 3.1 IN X CBF 1 SELECT 4 FROM * AS X' --await --basic-acl public-read-write
CID: 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m
...
$ dd if=/dev/urandom of=/tmp/object.sample.dev bs=6701 count=1
...
$ frostfs-cli -r s01.frostfs.devenv:8080 -c cfg.yml object put --cid 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m  --file /tmp/object.sample.dev --timeout 20m
 6701 / 6701 [===================================================================================================================================================] 100.00% 0s
[/tmp/object.sample.dev] Object successfully stored
  OID: 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB
  CID: 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m
$ 
$ frostfs-cli -r s01.frostfs.devenv:8080 -c cfg.yml object get --cid 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m --oid 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB --timeout 1h --file test.obj
 6701 / 6701 [===================================================================================================================================================] 100.00% 0s
[test.obj] Object successfully saved
ID: 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB
CID: 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m
Owner: NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM
...
Type: REGULAR
Attributes:
  FileName=object.sample.dev
...

Output for HEAD and HEAD with --raw:

$ frostfs-cli -r s01.frostfs.devenv:8080 -c cfg.yml object head --cid 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m --oid 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB --timeout 1h
ID: 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB
CID: 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m
Owner: NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM
... 
$ frostfs-cli -r s01.frostfs.devenv:8080 -c cfg.yml object head --cid 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m --oid 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB --timeout 1h --raw
Object is erasure-encoded, ec information received.
Total chunks: 4
  Index: 3 ID: EJgWQjbmPwYtUM7k41tUgq1xoAFJBMhRWwQ86iyQtLzH
  Index: 0 ID: AVmYce8qjMJj265eqrz8NM4NKMgMXHgz66tC1EQyMnJD
  Index: 2 ID: ALTUryGZvDU8m4veyAofSu5Zw8qy8DuWJDV8Y3RH85oW
  Index: 1 ID: DvbUABS5dPPPpgDyMV58W3LM1555qJt95kiRiRo5jwgq

Output for HEAD with --raw and one unavailable node:

$ docker stop s03
s03
$ frostfs-cli -r s01.frostfs.devenv:8080 -c cfg.yml object head --cid 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m --oid 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB --timeout 1h --raw
Object is erasure-encoded, ec information received.
Total chunks: 4
  Index: 3 ID: EJgWQjbmPwYtUM7k41tUgq1xoAFJBMhRWwQ86iyQtLzH
  Index: 2 ID: ALTUryGZvDU8m4veyAofSu5Zw8qy8DuWJDV8Y3RH85oW
  Index: 1 ID: DvbUABS5dPPPpgDyMV58W3LM1555qJt95kiRiRo5jwgq
$

Signed-off-by: Anton Nikiforov an.nikiforov@yadro.com

With these changes it is possible to get\head erasure-coded objects via cli. Example output for `GET`: ``` $ frostfs-cli -r s01.frostfs.devenv:8080 -c cfg.yml container create --policy 'EC 3.1 IN X CBF 1 SELECT 4 FROM * AS X' --await --basic-acl public-read-write CID: 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m ... $ dd if=/dev/urandom of=/tmp/object.sample.dev bs=6701 count=1 ... $ frostfs-cli -r s01.frostfs.devenv:8080 -c cfg.yml object put --cid 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m --file /tmp/object.sample.dev --timeout 20m 6701 / 6701 [===================================================================================================================================================] 100.00% 0s [/tmp/object.sample.dev] Object successfully stored OID: 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB CID: 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m $ $ frostfs-cli -r s01.frostfs.devenv:8080 -c cfg.yml object get --cid 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m --oid 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB --timeout 1h --file test.obj 6701 / 6701 [===================================================================================================================================================] 100.00% 0s [test.obj] Object successfully saved ID: 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB CID: 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m Owner: NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM ... Type: REGULAR Attributes: FileName=object.sample.dev ... ``` Output for `HEAD` and `HEAD` with `--raw`: ``` $ frostfs-cli -r s01.frostfs.devenv:8080 -c cfg.yml object head --cid 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m --oid 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB --timeout 1h ID: 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB CID: 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m Owner: NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM ... $ frostfs-cli -r s01.frostfs.devenv:8080 -c cfg.yml object head --cid 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m --oid 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB --timeout 1h --raw Object is erasure-encoded, ec information received. Total chunks: 4 Index: 3 ID: EJgWQjbmPwYtUM7k41tUgq1xoAFJBMhRWwQ86iyQtLzH Index: 0 ID: AVmYce8qjMJj265eqrz8NM4NKMgMXHgz66tC1EQyMnJD Index: 2 ID: ALTUryGZvDU8m4veyAofSu5Zw8qy8DuWJDV8Y3RH85oW Index: 1 ID: DvbUABS5dPPPpgDyMV58W3LM1555qJt95kiRiRo5jwgq ``` Output for `HEAD` with `--raw` and one unavailable node: ``` $ docker stop s03 s03 $ frostfs-cli -r s01.frostfs.devenv:8080 -c cfg.yml object head --cid 9ckAfG9FSsRZEmb6gJ8QJsiTxutQLgNvL9irRstNgV8m --oid 8VMcKs54FFiUKhigWhLWUtdimPU6qqX6TJVbXHXB23jB --timeout 1h --raw Object is erasure-encoded, ec information received. Total chunks: 4 Index: 3 ID: EJgWQjbmPwYtUM7k41tUgq1xoAFJBMhRWwQ86iyQtLzH Index: 2 ID: ALTUryGZvDU8m4veyAofSu5Zw8qy8DuWJDV8Y3RH85oW Index: 1 ID: DvbUABS5dPPPpgDyMV58W3LM1555qJt95kiRiRo5jwgq $ ``` Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
acid-ant force-pushed feature/ec-get-head from a7a14f0ff6 to ed7040b1dd 2024-04-22 10:33:57 +00:00 Compare
acid-ant force-pushed feature/ec-get-head from ed7040b1dd to ffd080219b 2024-04-22 12:20:15 +00:00 Compare
acid-ant force-pushed feature/ec-get-head from ffd080219b to 76777820cf 2024-04-23 06:05:58 +00:00 Compare
fyrchik requested changes 2024-04-23 06:39:52 +00:00
internal/logs/logs.go Outdated
@ -111,6 +116,7 @@ const (
GetCompletingTheOperation = "completing the operation"
GetRequestedObjectWasMarkedAsRemoved = "requested object was marked as removed"
GetRequestedObjectIsVirtual = "requested object is virtual"
GetRequestedObjectIsEC = "requested object is erasure coded"
fyrchik commented 2024-04-23 06:30:25 +00:00
Owner
Copy link

We have "erasure coded" here and "erasure-encoded" in CLI print.
Wikipedia uses "erasure-coded" https://en.wikipedia.org/wiki/Erasure_code, let's stick with it.

We have "erasure coded" here and "erasure-encoded" in CLI print. Wikipedia uses "erasure-coded" https://en.wikipedia.org/wiki/Erasure_code, let's stick with it.
acid-ant commented 2024-04-23 11:41:17 +00:00
Author
Member
Copy link

Agree, updated here in other places.

Agree, updated here in other places.
fyrchik marked this conversation as resolved
pkg/local_object_storage/engine/head.go Outdated
@ -109,3 +117,3 @@
return len(outEI.Chunks) == int(outEI.Chunks[0].Total)
case client.IsErrObjectAlreadyRemoved(err):
outError = err
fyrchik commented 2024-04-23 06:31:33 +00:00
Owner
Copy link

unrelated to the commit

unrelated to the commit
acid-ant commented 2024-04-23 11:40:58 +00:00
Author
Member
Copy link

This change required because we have check for amount of lines in method. Don't want to do refactoring in scope of this PR.

This change required because we have check for amount of lines in method. Don't want to do refactoring in scope of this PR.
fyrchik commented 2024-04-23 12:06:00 +00:00
Owner
Copy link

That is what I mean by unrelated, can we have a separate commit for it, before the EC changes?
Well, there is another whitespace change commit already.

That is what I mean by unrelated, can we have a separate commit for it, before the EC changes? Well, there is another whitespace change commit already.
acid-ant commented 2024-04-23 12:26:25 +00:00
Author
Member
Copy link

Ok, moved to the separate commit.

Ok, moved to the separate commit.
fyrchik marked this conversation as resolved
pkg/local_object_storage/metabase/get.go Outdated
@ -5,6 +5,8 @@ import (
"fmt"
"time"
"git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object"
fyrchik commented 2024-04-23 06:32:46 +00:00
Owner
Copy link

We try not to use api directly anywhere besides services, can we use SDK here?

We try not to use api directly anywhere besides services, can we use SDK here?
acid-ant commented 2024-04-23 11:39:52 +00:00
Author
Member
Copy link

Thanks, removed this import from here and another files. SDK updated accordingly.

Thanks, removed this import from here and another files. SDK updated accordingly.
fyrchik marked this conversation as resolved
pkg/services/object/get/assemblerec.go Outdated
@ -0,0 +44,4 @@
// Assemble assembles erasure coded object and writes it's content to ObjectWriter.
// It returns parent object.
func (a *assemblerec) Assemble(mainCtx context.Context, writer ObjectWriter, headOnly bool) (*objectSDK.Object, error) {
fyrchik commented 2024-04-23 06:36:35 +00:00
Owner
Copy link

When we are to add GET_RANGE do we need to alter this function or write code in another place?

When we are to add GET_RANGE do we need to alter this function or write code in another place?
acid-ant commented 2024-04-23 11:39:07 +00:00
Author
Member
Copy link

We need to extend assembler for EC a bit, yes.

We need to extend assembler for EC a bit, yes.
fyrchik marked this conversation as resolved
pkg/services/object/get/remote.go Outdated
@ -34,1 +36,4 @@
default:
r.log.Debug(logs.GetRemoteCallFailed, zap.Error(err))
if r.status == statusEC {
return len(r.infoEC.Chunks) == int(r.infoEC.Chunks[0].Total)
fyrchik commented 2024-04-23 06:37:18 +00:00
Owner
Copy link

What does this condition checks?

What does this condition checks?
acid-ant commented 2024-04-23 11:38:34 +00:00
Author
Member
Copy link

We need to continue to process the nodes in case of getting error from the node. Updated this line a bit.

We need to continue to process the nodes in case of getting error from the node. Updated this line a bit.
pkg/services/object/get/remote.go Outdated
@ -60,0 +66,4 @@
util.MergeECInfo(r.infoEC, errECInfo.ECInfo())
r.infoEC = errECInfo.ECInfo()
r.err = objectSDK.NewECInfoError(r.infoEC)
// TODO maybe we don't need to get all chunks here
fyrchik commented 2024-04-23 06:38:00 +00:00
Owner
Copy link

Each TODO should be accompanied by an issue, could you create one?

Each TODO should be accompanied by an issue, could you create one?
fyrchik commented 2024-04-23 06:38:41 +00:00
Owner
Copy link

Anyway, the condition here is easy to fix, the number of chunks we must have is equal to placementpolicy.ECDataCount()

Anyway, the condition here is easy to fix, the number of chunks we must have is equal to `placementpolicy.ECDataCount()`
acid-ant commented 2024-04-23 11:37:19 +00:00
Author
Member
Copy link

Fixed.

Fixed.
fyrchik marked this conversation as resolved
acid-ant force-pushed feature/ec-get-head from 76777820cf to c588a8793c 2024-04-23 11:37:13 +00:00 Compare
acid-ant force-pushed feature/ec-get-head from c588a8793c to cfa9932859 2024-04-23 12:25:13 +00:00 Compare
acid-ant force-pushed feature/ec-get-head from cfa9932859 to e9eec6c013 2024-04-24 10:51:51 +00:00 Compare
acid-ant force-pushed feature/ec-get-head from e9eec6c013 to 4dc52f023c 2024-04-24 11:04:07 +00:00 Compare
acid-ant changed title from WIP: node: Implement Get\Head requests for EC object to node: Implement Get\Head requests for EC object 2024-04-24 11:31:10 +00:00
acid-ant requested review from storage-core-committers 2024-04-24 11:31:27 +00:00
acid-ant requested review from storage-core-developers 2024-04-24 11:31:27 +00:00
dstepanov-yadro requested changes 2024-04-24 11:59:42 +00:00
pkg/services/object/get/assembleec.go Outdated
@ -0,0 +61,4 @@
var errRemovedRemote *apistatus.ObjectAlreadyRemoved
var errOutOfRangeRemote *apistatus.ObjectOutOfRange
var errRemovedLocal *apistatus.ObjectAlreadyRemoved
var errOutOfRangeLocal *apistatus.ObjectOutOfRange
dstepanov-yadro commented 2024-04-24 11:56:32 +00:00
Member
Copy link

Look like remote and local errors are the same.

Look like remote and local errors are the same.
acid-ant commented 2024-04-24 12:45:37 +00:00
Author
Member
Copy link

Removed.

Removed.
dstepanov-yadro commented 2024-04-24 14:30:46 +00:00
Member
Copy link

Incompleted

var errOutOfRangeRemote *apistatus.ObjectOutOfRange
var errOutOfRangeLocal *apistatus.ObjectOutOfRange
Incompleted ``` var errOutOfRangeRemote *apistatus.ObjectOutOfRange var errOutOfRangeLocal *apistatus.ObjectOutOfRange ```
acid-ant commented 2024-04-24 15:16:44 +00:00
Author
Member
Copy link

Sorry, fixed.

Sorry, fixed.
pkg/services/object/get/assemblerec.go Outdated
@ -0,0 +99,4 @@
}
obj, err := c.Reconstruct(parts)
if err == nil {
err = writer.WriteHeader(ctx, obj.CutPayload())
dstepanov-yadro commented 2024-04-24 11:56:01 +00:00
Member
Copy link

ctx -> mainCtx, or even better to move errgroup related code to separate func.
As far as I remember errgoup's ctx is cancelled after Wait.

ctx -> mainCtx, or even better to move `errgroup` related code to separate func. As far as I remember errgoup's ctx is cancelled after Wait.
👍 1
acid-ant commented 2024-04-24 12:45:19 +00:00
Author
Member
Copy link

Thanks a lot! Split on two functions.

Thanks a lot! Split on two functions.
acid-ant force-pushed feature/ec-get-head from 4dc52f023c to dd89b6cf14 2024-04-24 12:01:50 +00:00 Compare
acid-ant force-pushed feature/ec-get-head from dd89b6cf14 to 7802bf33ee 2024-04-24 12:44:28 +00:00 Compare
acid-ant force-pushed feature/ec-get-head from 7802bf33ee to 534b752134 2024-04-24 12:47:17 +00:00 Compare
acid-ant requested review from dstepanov-yadro 2024-04-24 13:30:39 +00:00
acid-ant force-pushed feature/ec-get-head from 534b752134 to a295c9075e 2024-04-24 13:31:16 +00:00 Compare
acid-ant force-pushed feature/ec-get-head from a295c9075e to 112a7c690f 2024-04-24 15:16:16 +00:00 Compare
fyrchik approved these changes 2024-04-24 15:49:28 +00:00
dstepanov-yadro approved these changes 2024-04-25 07:47:31 +00:00
fyrchik merged commit 112a7c690f into master 2024-04-25 11:57:06 +00:00
fyrchik referenced this pull request from a commit 2024-04-25 11:57:06 +00:00
[#1103] pre-commit: Exclude `*.svg`
Sign in to join this conversation.
Reviewers
No reviewers
fyrchik
dstepanov-yadro
TrueCloudLab/storage-core-developers
Labels
Clear labels   
P0

Highest

  
P1

High

  
P2

Medium

  
P3

Low

  
badger

Badger DB related issues

  
frostfs-adm

   
frostfs-cli

   
frostfs-ir

   
frostfs-lens

   
frostfs-node

   
good first issue

Good for newcomers

  
triage
  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff

  
blocked

The issue or PR is blocked by something

  
bug

Something is not working

  
config

Configuration format update or breaking change

  
discussion

Talking about something in order to reach a decision or to exchange ideas

  
documentation

Documentation related

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
go

Language features adoption

  
help wanted

Extra attention is needed

  
internal

   
invalid

Something is wrong

  
kludge

This is a kludge and we know it

  
observability

Related to metrics, tracing and logs

  
perfomance

Perfomance related

  
question

More information is needed

  
refactoring

Refactoring

  
wontfix

This won't be fixed

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
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#1103
No description provided.
Delete branch "acid-ant/frostfs-node:feature/ec-get-head"

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?