TrueCloudLab/frostfs-node

pilorama: Fix TreeDrop #805

Merged

fyrchik merged 1 commit from aarifullin/frostfs-node:fix/bforest_tree_drop into master

2023-11-17 10:21:39 +00:00

aarifullin commented

2023-11-15 09:07:06 +00:00

Member

If treeID is empty then deleting buckets for cursor may get
invalidated. So, buckets should be gathered before deleting.

Signed-off-by: Airat Arifullin a.arifullin@yadro.com

* If treeID is empty then deleting buckets for cursor may get invalidated. So, buckets should be gathered before deleting. Signed-off-by: Airat Arifullin <a.arifullin@yadro.com>

aarifullin added the

label 2023-11-15 09:07:06 +00:00

aarifullin force-pushed fix/bforest_tree_drop from 60ee316151 to c8a68155bf

2023-11-15 09:07:45 +00:00

Compare

aarifullin changed title from ~~[#XX] pilorama: Fix TreeDrop~~ to pilorama: Fix TreeDrop

2023-11-15 09:07:55 +00:00

aarifullin requested review from storage-core-committers 2023-11-15 09:09:41 +00:00

aarifullin requested review from storage-core-developers 2023-11-15 09:10:00 +00:00

dstepanov-yadro approved these changes 2023-11-15 09:15:06 +00:00

fyrchik reviewed 2023-11-15 09:28:38 +00:00

pkg/local_object_storage/pilorama/boltdb.go Outdated

					
				@ -1118,2 +1118,3 @@

							var buckets [][]byte

							for k, _ := c.Seek(prefix); k != nil && bytes.HasPrefix(k, prefix); k, _ = c.Next() {

								err := tx.DeleteBucket(k)

								buckets = append(buckets, k)

fyrchik commented

2023-11-15 09:28:36 +00:00

Owner

There is no need to create an intermediate slice, I believe we can just replace c.Next() with c.Seek(prefix)

There is no need to create an intermediate slice, I believe we can just replace `c.Next()` with `c.Seek(prefix)`

aarifullin commented

2023-11-15 10:32:32 +00:00

Author

Member

Agree. But it is needed to add cursor rewind: https://github.com/boltdb/bolt/issues/357#issuecomment-97851571: added it at the end of for body

Agree. But it is needed to add cursor rewind: https://github.com/boltdb/bolt/issues/357#issuecomment-97851571: added it at the end of `for` body

dstepanov-yadro commented

2023-11-15 14:55:59 +00:00

Member

Now it looks too complex IMHO. If I see this code in a week, I will be confused. @fyrchik , what's wrong with slice?

Now it looks too complex IMHO. If I see this code in a week, I will be confused. @fyrchik , what's wrong with slice?

fyrchik commented

2023-11-15 15:18:46 +00:00

Owner

Nothing wrong, it bugs me when I see 2 iterations instead of 1.
In this case buckets slice will generally be of size 1-2, may be nothing bad.

Nothing wrong, it bugs me when I see 2 iterations instead of 1. In this case `buckets` slice will generally be of size 1-2, may be nothing bad.

aarifullin force-pushed fix/bforest_tree_drop from c8a68155bf to 8d5300c41a

2023-11-15 10:30:04 +00:00

Compare

aarifullin requested review from dstepanov-yadro 2023-11-15 10:32:56 +00:00

dstepanov-yadro approved these changes 2023-11-16 07:09:23 +00:00

acid-ant approved these changes 2023-11-16 12:47:02 +00:00

fyrchik approved these changes 2023-11-17 10:21:28 +00:00

fyrchik merged commit 47286ebf32 into master

2023-11-17 10:21:39 +00:00

fyrchik referenced this pull request from a commit

2023-11-17 10:21:40 +00:00

[#805] pilorama: Fix TreeDrop

Sign in to join this conversation.

No reviewers

TrueCloudLab/storage-core-developers

No labels

good first issue

No milestone

No project

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#805

No description provided.