[#215] Fix get latest version node #215

r.loginov · 2023-09-28T09:46:41Z

r.loginov commented

2023-09-28 09:46:41 +00:00

We have the following problem. For example, if we created an object with the key a and an object with the key a/b (it is important that the name of the "folder" is the same as the name of the object), and then we call the aws rm --recursive command to the bucket in which they lie, then in the end the object with the key a it is not deleted.

This happens because when requesting a tree service to get a node of the object version, we get a node that is a secondary object a. The resulting node does not have an OID attribute, and as a result, further deletion of the object with the a key does not occur.

As a solution, an option is proposed in which we get the entire list of nodes by a certain path (key) and then choose the node of the object version we need ourselves, depending on the presence of the OID attribute and the timestamp value.

Signed-off-by: Roman Loginov r.loginov@yadro.com

We have the following problem. For example, if we created an object with the key `a` and an object with the key `a/b` (it is important that the name of the "folder" is the same as the name of the object), and then we call the `aws rm --recursive` command to the bucket in which they lie, then in the end the object with the key `a` it is not deleted. This happens because when [requesting](https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/src/commit/9f3119a875332436c270f46ff4c777ca479356ed/pkg/service/tree/tree.go#L592) a tree service to get a node of the object version, we get a node that is a secondary object `a`. The resulting node does not have an OID attribute, and as a result, further deletion of the object with the `a` key does not occur. As a solution, an option is proposed in which we get the entire list of nodes by a certain path (key) and then choose the node of the object version we need ourselves, depending on the presence of the OID attribute and the timestamp value. Signed-off-by: Roman Loginov <r.loginov@yadro.com>

r.loginov self-assigned this 2023-09-28 09:46:41 +00:00

r.loginov force-pushed feature/fix_get_latest_version_node from 9e1b398be1 to 9f3119a875

2023-09-28 09:57:06 +00:00

Compare

r.loginov requested review from storage-services-committers 2023-09-28 10:12:06 +00:00

r.loginov requested review from storage-services-developers 2023-09-28 10:12:07 +00:00

dkirillov reviewed 2023-09-29 06:31:18 +00:00

pkg/service/tree/tree.go Outdated

					
				@ -602,0 +608,4 @@

					)

					for i, node := range nodes {

						currentCreationTime := node.GetTimestamp()

						if len(node.GetMeta()) != 0 && currentCreationTime > maxCreationTime {

dkirillov commented

2023-09-29 06:31:18 +00:00

I'm not sure it's enough to check length of GetMeta (as I understand intermediate node also contains some meta: FileName). Probably it's better to check for OID attribute that must exist in case of object-version node

I'm not sure it's enough to check length of `GetMeta` (as I understand intermediate node also contains some meta: `FileName`). Probably it's better to check for `OID` attribute that must exist in case of object-version node

👍 1

dkirillov marked this conversation as resolved

r.loginov force-pushed feature/fix_get_latest_version_node from 9f3119a875 to 2ffb3e792e

2023-09-29 09:59:11 +00:00

Compare

dkirillov commented

2023-10-03 09:41:42 +00:00

Please, fix commit message (now it refers to #1162)

r.loginov force-pushed feature/fix_get_latest_version_node from 2ffb3e792e to dc8dc42f4f

2023-10-03 12:28:24 +00:00

Compare

r.loginov changed title from ~~[#1162] Fix get latest version node~~ to [#215] Fix get latest version node

2023-10-03 12:28:53 +00:00

dkirillov reviewed 2023-10-04 06:56:28 +00:00

pkg/service/tree/tree.go Outdated

					
				@ -602,0 +614,4 @@

						}

					}

					return nodes[targetIndexNode]

dkirillov commented

2023-10-04 06:56:28 +00:00

What if the nodes doesn't contain any node with OID attribute?

What if the `nodes` doesn't contain any node with `OID` attribute?

alexvanin commented

2023-10-04 11:03:18 +00:00

Let's write a unit test for getLatestNode and check there some corner cases here.

Let's write a unit test for `getLatestNode` and check there some corner cases here.

👍 1

r.loginov commented

2023-10-05 11:04:56 +00:00

What if the nodes doesn't contain any node with OID attribute?

Added a check of this case. Now in this case, the error layer.ErrNodeNotFound will be returned.

> What if the `nodes` doesn't contain any node with `OID` attribute? Added a check of this case. Now in this case, the error `layer.ErrNodeNotFound` will be returned.

dkirillov marked this conversation as resolved

alexvanin reviewed 2023-10-04 11:03:43 +00:00

alexvanin left a comment

Don't forget to open PR in support branch as well after.

r.loginov force-pushed feature/fix_get_latest_version_node from dc8dc42f4f to 2cd03d2fdd

2023-10-05 10:31:14 +00:00

Compare

alexvanin approved these changes 2023-10-05 14:55:57 +00:00

dkirillov reviewed 2023-10-06 06:36:05 +00:00

pkg/service/tree/tree.go Outdated

					
				@ -598,0 +606,4 @@

					var (

						maxCreationTime uint64

						targetIndexNode int

						objectNodeExist bool

dkirillov commented

2023-10-06 06:36:05 +00:00

If we initialize targetIndexNode with -1 then we can drop objectNodeExist variable and check for

if  targetIndexNode == -1 {
		return nil, layer.ErrNodeNotFound
}

But it's optional

If we initialize `targetIndexNode` with `-1` then we can drop `objectNodeExist` variable and check for ``` if targetIndexNode == -1 { return nil, layer.ErrNodeNotFound } ``` But it's optional

👍 1

dkirillov marked this conversation as resolved

dkirillov approved these changes 2023-10-06 06:36:12 +00:00

ironbee approved these changes 2023-10-06 07:28:13 +00:00

r.loginov force-pushed feature/fix_get_latest_version_node from 2cd03d2fdd to 9b8e991e0b

2023-10-06 07:31:52 +00:00

Compare

r.loginov requested review from dkirillov 2023-10-06 07:49:33 +00:00

dkirillov approved these changes 2023-10-06 07:57:48 +00:00

alexvanin merged commit e1ec61ddfc into master

2023-10-06 09:21:42 +00:00

alexvanin deleted branch feature/fix_get_latest_version_node