blobstor: Additional context of errors #2608

cthulhu-rider · 2023-10-03T08:14:47Z

Originally posted by @smallhive

warn	engine/engine.go:159	could not put object to shard	{"shard_id": "8TAArEwewr9p3aGppk4GVM", "error count": 24, "error": "could not put object to BLOB storage: operation not supported"}

codecov · 2023-10-03T08:46:12Z

Codecov Report

Attention: 37 lines in your changes are missing coverage. Please review.

Comparison is base (59a8198) 28.67% compared to head (6836165) 28.67%.

❗ Current head 6836165 differs from pull request most recent head 2ad8cfd. Consider uploading reports for the commit 2ad8cfd to get more accurate results

Files	Patch %	Lines
pkg/local_object_storage/blobstor/fstree/fstree.go	51.51%	14 Missing and 2 partials ⚠️
...ct_storage/blobstor/fstree/fstree_write_generic.go	0.00%	14 Missing ⚠️
...ject_storage/blobstor/fstree/fstree_write_linux.go	16.66%	4 Missing and 1 partial ⚠️
...kg/local_object_storage/blobstor/fstree/control.go	0.00%	1 Missing ⚠️
pkg/local_object_storage/blobstor/put.go	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2608      +/-   ##
==========================================
- Coverage   28.67%   28.67%   -0.01%     
==========================================
  Files         415      415              
  Lines       32234    32253      +19     
==========================================
+ Hits         9244     9249       +5     
- Misses      22195    22207      +12     
- Partials      795      797       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pkg/local_object_storage/blobstor/fstree/fstree.go

pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go

carpawell · 2023-10-03T11:40:47Z

pkg/local_object_storage/blobstor/fstree/fstree_write_generic.go

@@ -46,7 +46,7 @@ func (t *FSTree) writeData(p string, data []byte) error {
 	}

 	// unreachable, but precaution never hurts, especially 1 day before release.
-	return fmt.Errorf("couldn't read file after %d retries", retryCount)
+	return fmt.Errorf("couldn't write file after %d retries", retryCount)


wow, more like a bugfix for a separate commit

this is not a bug, just typo

logging "read" while writing is a bug to me. do not insist on a separate commit much

pkg/local_object_storage/blobstor/put.go

carpawell · 2023-10-23T07:31:55Z

Conflicts.

roman-khimov · 2023-11-13T20:19:27Z

Do we still need this?

pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go

carpawell · 2023-11-15T10:02:21Z

Do we still need this?

It has some little fixes. But should be rebased first to unserstand if it is good still.

Some errors may be too generic to detect the exact place they come from. Additional context helps. Signed-off-by: Leonard Lyubich <[email protected]>

After Go introduced `fs` package, error checkers from `os` packages may be replaced with more laconic `errors.Is` ones. An additional benefit is support for wrapped errors, which was not present in the old functions. Signed-off-by: Leonard Lyubich <[email protected]>

Type helps to realize what exact storage failed. Signed-off-by: Leonard Lyubich <[email protected]>

Previously, object command that opened dynamic session with storage node (like `put`) failed with `can't fetch current epoch: can't parse RPC endpoint: missing port in address` error when RPC endpoint was set in the config file and omitted in flags. Get network endpoint from viper box which contains values from config file. Refs #1574. Signed-off-by: Leonard Lyubich <[email protected]>

cthulhu-rider · 2023-11-15T10:53:23Z

rebased

roman-khimov

It becomes too chatty to me, but OK (there are some nices fixes there as well).

cthulhu-rider · 2023-11-15T12:27:12Z

@roman-khimov my goal is to avoid messagges like in the PR body when i can only imagine what the exact problem is

at the same time, i dont wanna force such changes cuz of usefule fixes only, so i can split fixes (we'll merge them now) and error context changes (can hang for now)

roman-khimov · 2023-11-15T12:29:44Z

Hanging changes are no good, they're either in or out.

cthulhu-rider force-pushed the fstree-err-ctx branch from 4b946de to b285958 Compare October 3, 2023 08:43

cthulhu-rider force-pushed the fstree-err-ctx branch 2 times, most recently from 977868e to 9e833e3 Compare October 3, 2023 09:18

roman-khimov reviewed Oct 3, 2023

View reviewed changes

pkg/local_object_storage/blobstor/fstree/fstree.go Outdated Show resolved Hide resolved

pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go Show resolved Hide resolved

cthulhu-rider force-pushed the fstree-err-ctx branch from 9e833e3 to f8eeef6 Compare October 3, 2023 11:23

cthulhu-rider marked this pull request as ready for review October 3, 2023 11:24

cthulhu-rider requested a review from carpawell as a code owner October 3, 2023 11:24

cthulhu-rider requested review from roman-khimov and AliceInHunterland October 3, 2023 11:24

carpawell reviewed Oct 3, 2023

View reviewed changes

carpawell reviewed Nov 15, 2023

View reviewed changes

pkg/local_object_storage/blobstor/fstree/fstree_write_linux.go Show resolved Hide resolved

cthulhu-rider force-pushed the fstree-err-ctx branch from f8eeef6 to 80358f1 Compare November 15, 2023 10:48

cthulhu-rider added 4 commits November 15, 2023 11:50

fstree: Add context to errors

e388e88

Some errors may be too generic to detect the exact place they come from. Additional context helps. Signed-off-by: Leonard Lyubich <[email protected]>

blobstor: Extend Put error context with sub-storage type

c00a75e

Type helps to realize what exact storage failed. Signed-off-by: Leonard Lyubich <[email protected]>

cthulhu-rider force-pushed the fstree-err-ctx branch from 80358f1 to 2ad8cfd Compare November 15, 2023 10:51

cthulhu-rider requested a review from carpawell November 15, 2023 11:02

roman-khimov approved these changes Nov 15, 2023

View reviewed changes

roman-khimov added this to the v0.39.0 milestone Nov 15, 2023

carpawell approved these changes Nov 15, 2023

View reviewed changes

roman-khimov merged commit c673b51 into master Nov 15, 2023
8 checks passed

roman-khimov deleted the fstree-err-ctx branch November 15, 2023 13:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

blobstor: Additional context of errors #2608

blobstor: Additional context of errors #2608

cthulhu-rider commented Oct 3, 2023

codecov bot commented Oct 3, 2023 •

edited

Loading

carpawell Oct 3, 2023

cthulhu-rider Oct 5, 2023

carpawell Oct 23, 2023

carpawell commented Oct 23, 2023

roman-khimov commented Nov 13, 2023

carpawell commented Nov 15, 2023

cthulhu-rider commented Nov 15, 2023

roman-khimov left a comment

cthulhu-rider commented Nov 15, 2023 •

edited

Loading

roman-khimov commented Nov 15, 2023

blobstor: Additional context of errors #2608

blobstor: Additional context of errors #2608

Conversation

cthulhu-rider commented Oct 3, 2023

codecov bot commented Oct 3, 2023 • edited Loading

Codecov Report

carpawell Oct 3, 2023

Choose a reason for hiding this comment

cthulhu-rider Oct 5, 2023

Choose a reason for hiding this comment

carpawell Oct 23, 2023

Choose a reason for hiding this comment

carpawell commented Oct 23, 2023

roman-khimov commented Nov 13, 2023

carpawell commented Nov 15, 2023

cthulhu-rider commented Nov 15, 2023

roman-khimov left a comment

Choose a reason for hiding this comment

cthulhu-rider commented Nov 15, 2023 • edited Loading

roman-khimov commented Nov 15, 2023

codecov bot commented Oct 3, 2023 •

edited

Loading

cthulhu-rider commented Nov 15, 2023 •

edited

Loading