EC put #1064

Merged
fyrchik merged 5 commits from dstepanov-yadro/frostfs-node:feat/ec_put into master 2024-04-09 07:08:55 +00:00
  • put unprepared objects to EC container
  • put prepared objects to EC container is not supported because frostfs-node requires user's private key for EC chunks
  • policer skips EC containers
  • frostfs-cli outputs EC header
  • added VSCode 4 storage nodes + IR configuration
- put unprepared objects to EC container - put prepared objects to EC container is not supported because frostfs-node requires user's private key for EC chunks - policer skips EC containers - frostfs-cli outputs EC header - added VSCode `4 storage nodes + IR` configuration
dstepanov-yadro added 6 commits 2024-03-29 13:48:16 +00:00
71b13ba0bb [#9999] mod: Bump sdk-go and api-go version
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
ac6becb827 [#9999] putsvc: Refactor distributed target
Extract object builder.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
bc27bd6e74 [#9999] putsvc: Add EC put
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
0ff42fe9d4 [#9999] dev: Add `IR + 4 storage nodes` configuration
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
f48ede44a2 [#9999] cli: Add EC header output to object head
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
DCO action / DCO (pull_request) Successful in 1m11s Details
Vulncheck / Vulncheck (pull_request) Successful in 3m27s Details
Tests and linters / Staticcheck (pull_request) Failing after 4m32s Details
Build / Build Components (1.21) (pull_request) Successful in 4m18s Details
Build / Build Components (1.20) (pull_request) Successful in 4m27s Details
Tests and linters / gopls check (pull_request) Failing after 6m17s Details
Tests and linters / Lint (pull_request) Successful in 6m59s Details
Tests and linters / Tests (1.21) (pull_request) Failing after 8m29s Details
Tests and linters / Tests with -race (pull_request) Failing after 10m14s Details
Tests and linters / Tests (1.20) (pull_request) Failing after 10m32s Details
b9fdf9afd0
[#9999] policer: Disable EC processing
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
dstepanov-yadro force-pushed feat/ec_put from b9fdf9afd0 to 553e43a37e 2024-03-29 13:52:02 +00:00 Compare
dstepanov-yadro force-pushed feat/ec_put from 553e43a37e to 8b59030e08 2024-03-29 14:00:40 +00:00 Compare
dstepanov-yadro force-pushed feat/ec_put from 8b59030e08 to 3d61db3986 2024-04-01 09:29:09 +00:00 Compare
dstepanov-yadro force-pushed feat/ec_put from 3d61db3986 to 47434d621c 2024-04-01 09:30:53 +00:00 Compare
dstepanov-yadro force-pushed feat/ec_put from 47434d621c to 4eac25598f 2024-04-01 12:19:24 +00:00 Compare
dstepanov-yadro force-pushed feat/ec_put from 4eac25598f to a83dfa4f01 2024-04-01 12:26:55 +00:00 Compare
dstepanov-yadro force-pushed feat/ec_put from a83dfa4f01 to 1ded16ab9a 2024-04-01 12:27:35 +00:00 Compare
dstepanov-yadro force-pushed feat/ec_put from 1ded16ab9a to da3a157104 2024-04-01 12:37:09 +00:00 Compare
dstepanov-yadro force-pushed feat/ec_put from da3a157104 to 98350bad84 2024-04-01 12:41:38 +00:00 Compare
dstepanov-yadro changed title from WIP: EC put to EC put 2024-04-01 12:45:03 +00:00
dstepanov-yadro requested review from storage-core-committers 2024-04-01 12:47:18 +00:00
dstepanov-yadro requested review from storage-core-developers 2024-04-01 12:49:17 +00:00
fyrchik requested changes 2024-04-01 13:22:48 +00:00
@ -58,11 +58,13 @@ It will be stored in sidechain when inner ring will accepts it.`,
"use --force option to skip this check: %w", err)
for i, nodes := range nodesByRep {
//lint:ignore SA1019 will be fixed later

Do we have a task or will it be done in this PR?

Do we have a task or will it be done in this PR?
Poster
Collaborator

fixed

fixed
fyrchik marked this conversation as resolved
@ -0,0 +45,4 @@
}
if !placement.IsECSupported(obj) {
// must be resolved by caller

What do you mean by this comment?

What do you mean by this comment?
Poster
Collaborator

ec_writer supports only regular objects. Linking objects, tombstones and locks write should be handled by defaultWriter (aka repWriter).
The current implementation satisfies this condition. It's just an additional check and foolproof.

`ec_writer` supports only regular objects. Linking objects, tombstones and locks write should be handled by `defaultWriter` (aka `repWriter`). The current implementation satisfies this condition. It's just an additional check and foolproof.
fyrchik marked this conversation as resolved
@ -0,0 +211,4 @@
// start node from according part index
// but try others if it fails
var err error
for i := 0; i < len(nodes); i++ {

We can't try all others, because this violates the assumption of how many nodes can fail. With this code it could happen that all chunks reside on one node.

I suggest to fail here: we already fail in a similar way for big objects (so there could be garbage).
Later we can improve this (probably i < len(nodes) should be replaced with something else, but not i < parityCount, so this is not obvious).

We can't try _all_ others, because this violates the assumption of how many nodes can fail. With this code it could happen that all chunks reside on one node. I suggest to fail here: we already fail in a similar way for big objects (so there could be garbage). Later we can improve this (probably `i < len(nodes)` should be replaced with something else, but not `i < parityCount`, so this is not obvious).
Poster
Collaborator

fixed

fixed
fyrchik marked this conversation as resolved
@ -247,0 +258,4 @@
return &multiObjectWriter{
ecWriter: &ecWriter{
cfg: p.cfg,
placementOpts: append(prm.traverseOpts, placement.WithCopyNumbers(nil)), // copies number ignored for EC

What happens if I have EC 2+1 and 4 nodes in the container?

What happens if I have `EC 2+1` and 4 nodes in the container?
Poster
Collaborator

What exactly are you asking? EC chunks will be saved on 3 of 4 container nodes.

What exactly are you asking? EC chunks will be saved on 3 of 4 container nodes.
@ -0,0 +7,4 @@
)
// IsECPlacement returns True if policy is erasure coding policy.
func IsECPlacement(policy netmapSDK.PlacementPolicy) bool {

These functions don't seem to be related to placement (and thus to this package) (especially ECDataCount which seems a rather narrow-scoped helper), why are they here?

These functions don't seem to be related to placement (and thus to this package) (especially `ECDataCount` which seems a rather narrow-scoped helper), why are they here?
Poster
Collaborator

I don't think these functions don't seem to be related to placement. I think there are about placement.

I don't think these functions don't seem to be related to placement. I think there are about placement.
Poster
Collaborator

moved to core

moved to `core`
fyrchik marked this conversation as resolved
dstepanov-yadro force-pushed feat/ec_put from 98350bad84 to f17686fc29 2024-04-01 14:11:24 +00:00 Compare
aarifullin reviewed 2024-04-01 15:44:16 +00:00
@ -0,0 +12,4 @@
type multiObjectWriter struct {
ecWriter transformer.ObjectWriter
repWriter transformer.ObjectWriter
Collaborator
  1. I don't see any reason to keep repWriter within multiObjectWriter at least because you don't use newECWriter if container does not support EC policy. So, it seems it always skips m.repWriter.WriteObject(ctx, obj). Please, fix me, if I am incorrect

  2. multi- prefix sounds like the object writer is able to choose how to write an object to a container: either by EC placement or by REP although a container adheres to single placement. What do you think about the idea to use objectWriterDispatcher name instead?

1. I don't see any reason to keep `repWriter` within `multiObjectWriter` at least because you don't use `newECWriter` if container does not support EC policy. So, it seems it always skips `m.repWriter.WriteObject(ctx, obj)`. Please, fix me, if I am incorrect 2. `multi-` prefix sounds like the object writer is able to *choose* how to write an object to a container: either by `EC` placement or by `REP` although a container adheres to single placement. What do you think about the idea to use `objectWriterDispatcher` name instead?
Poster
Collaborator
  1. EC container stores some objects (tombstones, locks, linking objects) as regular objects: these objects must be saved on every container node without EC splitting.
  2. done
1. EC container stores some objects (tombstones, locks, linking objects) as regular objects: these objects must be saved on every container node without EC splitting. 2. done
Collaborator

EC container stores some objects (tombstones, locks, linking objects) as regular objects: these objects must be saved on every container node without EC splitting.

Thanks for explanation

UPD: btw, I incorrectly read your code

if container.IsECContainer(prm.cnr) && object.IsECSupported(prm.hdr)

I didn't notice you pass header to IsECSupported - that is why I've got confused. But for now it is OK 👍

> EC container stores some objects (tombstones, locks, linking objects) as regular objects: these objects must be saved on every container node without EC splitting. Thanks for explanation UPD: btw, I incorrectly read your code ```go if container.IsECContainer(prm.cnr) && object.IsECSupported(prm.hdr) ``` I didn't notice you pass header to `IsECSupported` - that is why I've got confused. But for now it is OK 👍
aarifullin marked this conversation as resolved
dstepanov-yadro force-pushed feat/ec_put from f17686fc29 to 7d1ce328cf 2024-04-01 15:55:08 +00:00 Compare
aarifullin approved these changes 2024-04-01 17:57:53 +00:00
acid-ant approved these changes 2024-04-03 11:39:53 +00:00
dstepanov-yadro force-pushed feat/ec_put from 7d1ce328cf to 04a23efef6 2024-04-03 12:35:27 +00:00 Compare

image

![image](/attachments/a0e97ea9-955f-4ead-96b6-550797d3c40d)
fyrchik added this to the v0.39.0 milestone 2024-04-09 07:04:00 +00:00
fyrchik approved these changes 2024-04-09 07:08:13 +00:00
fyrchik merged commit f5b67c6735 into master 2024-04-09 07:08:55 +00:00
fyrchik referenced this issue from a commit 2024-04-09 07:08:57 +00:00
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
4 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#1064
There is no content yet.