TrueCloudLab/frostfs-node
19
1
Fork 27
Code Issues 91 Pull requests 7 Projects Releases 1 Activity Actions

Do not change shard mode to DEGRADED_READ_ONLY in case of no space left from blobovnicza #1166

Merged
fyrchik merged 1 commit from dstepanov-yadro/frostfs-node:fix/out_of_space_dro into master 2024-09-04 19:51:09 +00:00
Conversation 0 Commits 1 Files changed 1 +4 -1
1 changed files with 4 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
pkg/local_object_storage/blobstor/blobovniczatree/put.go
View file

@ -2,6 +2,7 @@ package blobovniczatree
import (
"context"
"errors"
"path/filepath"
"time"
@ -108,7 +109,9 @@ func (i *putIterator) iterate(ctx context.Context, lvlPath string) (bool, error)
zap.String("error", err.Error()),
zap.String("trace_id", tracingPkg.GetTraceID(ctx)))
}
if errors.Is(err, blobovnicza.ErrNoSpace) {
i.AllFull = true
fyrchik commented 2024-06-10 07:57:08 +00:00
Review
Copy link

Again, do we exit if we received this error from at least 1 blobovnicza? Until we have vacuum I think it is not worth having this optimization, as others blobovniczas may still have free pages.

Again, do we exit if we received this error from at least 1 blobovnicza? Until we have vacuum I think it is not worth having this optimization, as others blobovniczas may still have free pages.
dstepanov-yadro commented 2024-06-10 08:23:45 +00:00
Review
Copy link

No, blobstor will try all databases:

	i.AllFull = false <------- here AllFull resets

	_, err = active.Blobovnicza().Put(ctx, i.PutPrm)
	if err != nil {
		if !isLogical(err) {
			i.B.reportError(logs.BlobovniczatreeCouldNotPutObjectToActiveBlobovnicza, err)
		} else {
			i.B.log.Debug(logs.BlobovniczatreeCouldNotPutObjectToActiveBlobovnicza,
				zap.String("path", active.SystemPath()),
				zap.String("error", err.Error()),
				zap.String("trace_id", tracingPkg.GetTraceID(ctx)))
		}

		if errors.Is(err, blobovnicza.ErrNoSpace) {
			i.AllFull = true
No, blobstor will try all databases: ``` i.AllFull = false <------- here AllFull resets _, err = active.Blobovnicza().Put(ctx, i.PutPrm) if err != nil { if !isLogical(err) { i.B.reportError(logs.BlobovniczatreeCouldNotPutObjectToActiveBlobovnicza, err) } else { i.B.log.Debug(logs.BlobovniczatreeCouldNotPutObjectToActiveBlobovnicza, zap.String("path", active.SystemPath()), zap.String("error", err.Error()), zap.String("trace_id", tracingPkg.GetTraceID(ctx))) } if errors.Is(err, blobovnicza.ErrNoSpace) { i.AllFull = true ```
fyrchik commented 2024-06-10 10:07:26 +00:00
Review
Copy link

I don't understand, why do we need this change in this PR? Is something wrong without it?

I don't understand, why do we need this change in this PR? Is something wrong without it?
dstepanov-yadro commented 2024-06-10 10:38:04 +00:00
Review
Copy link

Without this change blobovnicza tree will return non logical error:

Line 64 in a0c5882
return common.PutRes{}, errPutFailed


So shard will increase error counter.

Without this change blobovnicza tree will return non logical error: https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/a0c588263bd550bb131a8ed2f1a5ad318811018c/pkg/local_object_storage/blobstor/blobovniczatree/put.go#L64 So shard will increase error counter.
fyrchik commented 2024-06-10 11:28:25 +00:00
Review
Copy link

Hm, but why iterateDeepest return non-nil error?

Hm, but why `iterateDeepest` return non-nil error?
dstepanov-yadro commented 2024-06-10 11:43:09 +00:00
Review
Copy link

By design.

By design.
}
return false, nil
}