Fix checking EC parent existence on Put object to shard #1548

dstepanov-yadro · 2024-12-10T10:03:22Z

dstepanov-yadro commented

2024-12-10 10:03:22 +00:00

Case:

EC object saved to container with EC3.1 policy
Policer lists EC chunk on node1
User deletes EC object
Policer performs HEAD request with EC parent object ID from node1 to node2, node2 returns object already removed error
Policer performs PUT request with current node's EC chunk to adjust EC placement
Node2 saves EC chunk from node1

Result: there is only one EC chunk left in the cluster, but user can't delete it.

Fixes:

Policer doesn't try to mode chunk to other node in case of already removed error
If EC parent object id is specified, metabase checks that EC parent object is not expired or already removed

Case: 1. EC object saved to container with EC3.1 policy 2. Policer lists EC chunk on node1 3. User deletes EC object 4. Policer performs `HEAD` request with EC parent object ID from node1 to node2, node2 returns `object already removed` error 5. Policer performs `PUT` request with current node's EC chunk to adjust EC placement 6. Node2 saves EC chunk from node1 Result: there is only one EC chunk left in the cluster, but user can't delete it. Fixes: 1. Policer doesn't try to mode chunk to other node in case of `already removed` error 2. If EC parent object id is specified, metabase checks that EC parent object is not expired or already removed

dstepanov-yadro added 3 commits 2024-12-10 10:03:23 +00:00

[#9999 ] policer: Do not replicate EC chunk if object already removed 1718e0de5f

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>

[#9999 ] engine: Rename parent -> ecParent c755a2fac9

Parent could mean split parent or EC parent. In this case it is EC parent only.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>

[#9999 ] metabase: Check if EC parent is removed or expired

Tests and linters / Run gofumpt (pull_request) Successful in 3m22s

Details

DCO action / DCO (pull_request) Successful in 3m46s

Details

Pre-commit hooks / Pre-commit (pull_request) Successful in 3m49s

Details

Vulncheck / Vulncheck (pull_request) Successful in 3m51s

Details

Tests and linters / gopls check (pull_request) Successful in 4m53s

Details

Build / Build Components (pull_request) Successful in 5m2s

Details

Tests and linters / Staticcheck (pull_request) Successful in 5m27s

Details

Tests and linters / Lint (pull_request) Successful in 5m44s

Details

Tests and linters / Tests (pull_request) Successful in 7m31s

Details

Tests and linters / Tests with -race (pull_request) Successful in 7m35s

Details

135245f378

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>

dstepanov-yadro force-pushed fix/path_removed_ec from 135245f378 to 169df9004d

2024-12-10 10:04:31 +00:00

Compare

dstepanov-yadro changed title from ~~WIP: Fix checking EC parent existance on Put object to shard~~ to Fix checking EC parent existance on Put object to shard

2024-12-10 10:34:26 +00:00

dstepanov-yadro requested review from storage-core-committers 2024-12-10 10:34:44 +00:00

dstepanov-yadro requested review from storage-core-developers 2024-12-10 10:34:45 +00:00

dstepanov-yadro changed title from ~~Fix checking EC parent existance on Put object to shard~~ to Fix checking EC parent existence on Put object to shard

2024-12-10 10:35:00 +00:00

a-savchuk reviewed 2024-12-10 11:12:40 +00:00

pkg/services/policer/ec.go Outdated

					
				@ -281,6 +281,8 @@ func (p *Policer) adjustECPlacement(ctx context.Context, objInfo objectcore.Info

								}

								chunkIDs[ch.Index] = ecInfoChunkID

							}

						} else if client.IsErrObjectAlreadyRemoved(err) {

a-savchuk commented

2024-12-10 11:12:40 +00:00

I think I've suggested similar changes for REP policy handling (#1543), but #1543 (comment) was said to me.

@fyrchik Could you please comment?

I think I've suggested similar changes for `REP` policy handling (https://git.frostfs.info/TrueCloudLab/frostfs-node/issues/1543), but https://git.frostfs.info/TrueCloudLab/frostfs-node/issues/1543#issuecomment-60284 was said to me. @fyrchik Could you please comment?

dstepanov-yadro commented

2024-12-10 11:37:59 +00:00

ok, fixed

fyrchik commented

2024-12-10 12:26:32 +00:00

What exactly was fixed? Is seems you still introduce a new behaviour:

What exactly was fixed? Is seems you still introduce a new behaviour: ![image](/attachments/14b33afa-e25f-4fc4-b3b8-5f1deb352215)

image.png

52 KiB

fyrchik commented

2024-12-10 12:27:26 +00:00

Sorry, the behaviour is the same, why have this commit at all?

dstepanov-yadro commented

2024-12-10 12:49:23 +00:00

Technically this commit relates to policer, but others relate to engine.
If the actual question is why this change, then this change if required to not to perform PUT request as we know, that object already removed and PUT request will fail anyway.

Technically this commit relates to policer, but others relate to engine. If the actual question is why this change, then this change if required to not to perform PUT request as we know, that object already removed and PUT request will fail anyway.