EC GET #3497

cthulhu-rider · 2025-08-01T12:45:19Z

based on Implement EC logic in PUT #3457 in terms of format

codecov · 2025-08-11T12:15:30Z

Codecov Report

❌ Patch coverage is 60.79404% with 158 lines in your changes missing coverage. Please review.
✅ Project coverage is 25.96%. Comparing base (adf5444) to head (db69124).

Files with missing lines	Patch %	Lines
pkg/services/object/get/ec.go	65.68%	69 Missing and 24 partials ⚠️
pkg/services/object/get/util.go	33.84%	40 Missing and 3 partials ⚠️
cmd/neofs-node/object.go	0.00%	9 Missing ⚠️
pkg/services/object/get/container.go	56.25%	5 Missing and 2 partials ⚠️
internal/ec/ec.go	50.00%	2 Missing and 1 partial ⚠️
pkg/services/object/get/service.go	0.00%	2 Missing ⚠️
cmd/neofs-node/policy.go	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3497      +/-   ##
==========================================
+ Coverage   25.66%   25.96%   +0.30%     
==========================================
  Files         659      660       +1     
  Lines       49373    49755     +382     
==========================================
+ Hits        12673    12921     +248     
- Misses      35694    35799     +105     
- Partials     1006     1035      +29

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

carpawell

Will there be issues for TODOs or some separate PR that fixes them? We once had work that was only related to going through every TODO to fix it or create a new issue for it.

pkg/services/object/get/ec.go

carpawell · 2025-08-29T08:28:44Z

pkg/services/object/get/ec.go

+		return object.Object{}, nil, errors.New("all nodes failed")
+	}
+
+	defer rc.Close()


can this panic in theory, if for above is empty?

only in theory, if some expectations of behavior are violated

pkg/services/object/get/ec.go

carpawell · 2025-08-29T08:31:01Z

pkg/services/object/get/ec.go

+		return *parentHdr, nil, nil
+	}
+
+	buf := make([]byte, partHdr.PayloadSize())


can it be gigabytes?

it can be up to MaxObjectSize net setting

oh, missed that there are part and parent headers (confusing naming btw, at least, i misunderstood)

it can be up to MaxObjectSize net setting

ok, but even in this case, is it ok to always allocate so much memory? found TODOs about streaming but isn't it necessary to solve ASAP?

isn't it necessary to solve ASAP

no. And not only due to not yet resolved streaming issue, but cuz decoder can now work in-mem only (#3501)

pkg/services/object/get/ec.go

pkg/services/object/get/service.go

cthulhu-rider · 2025-08-29T11:18:43Z

Will there be issues for TODOs or some separate PR that fixes them? We once had work that was only related to going through every TODO to fix it or create a new issue for it.

all TODOs will be resolved later. There are many of them, so i dont wanna create issue for each. Prefer to just have them in code

carpawell · 2025-08-29T16:23:51Z

Prefer to just have them in code

Dont mind, just wanted to hear that it is not a final solution and the next iteration will be soon.

roman-khimov

Unaddressed TODOs should have corresponding issues.

cmd/neofs-node/policy_test.go

internal/ec/ec.go

pkg/services/object/get/get.go

pkg/services/object/get/service.go

pkg/services/object/get/util.go

pkg/services/object/get/ec.go

Follow cd3e5d3 for GET. Closes #3422. Signed-off-by: Leonard Lyubich <[email protected]>

Signed-off-by: Leonard Lyubich <[email protected]>

roman-khimov · 2025-09-05T11:48:55Z

pkg/services/object/get/ec.go

+			return object.Object{}, err
+		}
+
+		fmt.Println(err)


carpawell

Except fmt.Println.

cthulhu-rider force-pushed the ec-get branch 7 times, most recently from 1a25190 to 340a149 Compare August 11, 2025 12:12

cthulhu-rider force-pushed the ec-get branch 4 times, most recently from 17e09c9 to 7f286bf Compare August 12, 2025 12:29

This was referenced Aug 14, 2025

services/object: use stream API for Get operation #3466

Merged

Metabase EC #3530

Merged

cthulhu-rider force-pushed the ec-get branch 7 times, most recently from 689dbc4 to 243ea9c Compare August 26, 2025 14:19

cthulhu-rider marked this pull request as ready for review August 26, 2025 14:50

cthulhu-rider requested review from roman-khimov, carpawell and End-rey as code owners August 26, 2025 14:50

cthulhu-rider force-pushed the ec-get branch from 243ea9c to c468302 Compare August 28, 2025 15:07

carpawell reviewed Aug 29, 2025

View reviewed changes

cthulhu-rider force-pushed the ec-get branch from c468302 to 9c520f3 Compare August 29, 2025 11:14

cthulhu-rider force-pushed the ec-get branch from 9c520f3 to d8c4555 Compare September 1, 2025 09:06

cthulhu-rider requested a review from carpawell September 1, 2025 09:23

cthulhu-rider force-pushed the ec-get branch from d8c4555 to 3779e74 Compare September 1, 2025 09:24

roman-khimov reviewed Sep 5, 2025

View reviewed changes

cthulhu-rider mentioned this pull request Sep 5, 2025

Split context in EC GET #3560

Open

cthulhu-rider force-pushed the ec-get branch 5 times, most recently from 52f7c5a to 64bd139 Compare September 5, 2025 10:43

cthulhu-rider added 2 commits September 5, 2025 13:51

sn/object: Initial support for erasure coding policies by GET server

f2557a6

Follow cd3e5d3 for GET. Closes #3422. Signed-off-by: Leonard Lyubich <[email protected]>

sn/object: Fix interface docs

db69124

Signed-off-by: Leonard Lyubich <[email protected]>

cthulhu-rider force-pushed the ec-get branch from 64bd139 to db69124 Compare September 5, 2025 10:51

roman-khimov reviewed Sep 5, 2025

View reviewed changes

pkg/services/object/get/ec.go

return object.Object{}, err

}

fmt.Println(err)

Copy link

Member

roman-khimov Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

carpawell reacted with eyes emoji

carpawell approved these changes Sep 5, 2025

View reviewed changes

EC GET #3497

Are you sure you want to change the base?

EC GET #3497

Conversation

cthulhu-rider commented Aug 1, 2025

Uh oh!

codecov bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

carpawell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

carpawell Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

cthulhu-rider Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

carpawell Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

cthulhu-rider Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

carpawell Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

cthulhu-rider Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cthulhu-rider commented Aug 29, 2025

Uh oh!

carpawell commented Aug 29, 2025

Uh oh!

roman-khimov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

roman-khimov Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

carpawell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Aug 11, 2025 •

edited

Loading