Skip to content

Conversation

@cthulhu-rider
Copy link
Contributor

@cthulhu-rider cthulhu-rider commented Jul 8, 2025

No description provided.

@cthulhu-rider cthulhu-rider force-pushed the ec-place branch 2 times, most recently from 54df907 to bb9781b Compare July 9, 2025 16:25
@cthulhu-rider cthulhu-rider force-pushed the ec-place branch 8 times, most recently from 4e3eb97 to dba5703 Compare July 15, 2025 14:58
@codecov
Copy link

codecov bot commented Jul 15, 2025

Codecov Report

❌ Patch coverage is 72.57143% with 96 lines in your changes missing coverage. Please review.
✅ Project coverage is 24.20%. Comparing base (0a90006) to head (cd3e5d3).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
pkg/services/object/put/ec.go 55.29% 27 Missing and 11 partials ⚠️
pkg/services/object/put/distributed.go 55.84% 27 Missing and 7 partials ⚠️
internal/ec/ec.go 62.50% 8 Missing and 4 partials ⚠️
pkg/services/object/put/streamer.go 66.66% 7 Missing and 4 partials ⚠️
cmd/neofs-node/object.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3457      +/-   ##
==========================================
+ Coverage   23.93%   24.20%   +0.27%     
==========================================
  Files         669      677       +8     
  Lines       50251    50538     +287     
==========================================
+ Hits        12028    12235     +207     
- Misses      37257    37312      +55     
- Partials      966      991      +25     

☔ 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.

@cthulhu-rider cthulhu-rider force-pushed the ec-place branch 3 times, most recently from ea251ef to dfebffd Compare July 15, 2025 15:19
@cthulhu-rider cthulhu-rider changed the title EC partitioning (placement) Implement EC logic in PUT Jul 15, 2025
@cthulhu-rider cthulhu-rider marked this pull request as ready for review July 15, 2025 15:27
@cthulhu-rider cthulhu-rider marked this pull request as draft July 16, 2025 07:07
@cthulhu-rider cthulhu-rider force-pushed the ec-place branch 6 times, most recently from f88f10e to cd56a48 Compare July 17, 2025 07:42
@cthulhu-rider cthulhu-rider marked this pull request as ready for review July 17, 2025 08:21
@roman-khimov roman-khimov requested a review from Copilot July 18, 2025 12:05
return nil, fmt.Errorf("restore Reed-Solomon: %w", err)
}

// TODO: last part may be shorter, do not overallocate buffer.
Copy link
Member

Choose a reason for hiding this comment

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

I doubt it's a problem practically, just a tiny rounding error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thinkin about parts of several MB len with the tiny last one (1B in particular)

Copy link
Member

Choose a reason for hiding this comment

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

But aren't EC part all ~same size? The difference I expect here is on the order of NUM_OF_PARTS bytes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they all exactly the same size. So the last part may require alignment. And it may be quite large

// header. Returns [ErrAttributeNotFound] if the attribute is missing.
//
// GetIntAttribute ignores all attribute values except the first.
func GetIntAttribute(hdr object.Object, attr string) (int, error) {
Copy link
Member

Choose a reason for hiding this comment

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

This wants to be in SDK, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, eventually. We can consider all general-purpose internal/ parts as candidates for moving to SDK. But doing this right away is inconvenient due to modules

roman-khimov added a commit that referenced this pull request Aug 8, 2025
I think #3505 and #3507 and bad enough for mainnet to justify a patch
release before we merge #3457 (which is the next in queue, but better
not be added into `.2`).
@cthulhu-rider cthulhu-rider force-pushed the ec-place branch 6 times, most recently from 1a5f7ce to 2917378 Compare August 11, 2025 11:55
}
} else {
err = t.placementIterator.iterateNodesForObject(id, t.sendObject)
broadcast := tombOrLink || (!t.localOnly && typ == objectSDK.TypeLock) || len(t.obj.Children()) > 0
Copy link
Member

Choose a reason for hiding this comment

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

Makes me wonder about localOnly && tombOrLink, but at least that's the way it was done previously.

@roman-khimov roman-khimov added the blocked Can't be done because of something label Aug 12, 2025
Use one instruction to calculate the flag. Also, do it per-object and do
not store in the struct field. In addition, evaluate only for non-local
requests.

Signed-off-by: Leonard Lyubich <[email protected]>
Will be useful for EC policies in #3420.

Signed-off-by: Leonard Lyubich <[email protected]>
So it can be reused.

Signed-off-by: Leonard Lyubich <[email protected]>
This extends PUT server logic to comply with EC policies - independently
or together with REP ones. This logic will remain inactive until it
becomes possible to create containers with EC policies.

This works for small objects only: encoding and distribution will be the
same for big objects, but SNs will deny them without proper adaptation.

Closes #3419. Refs #3420.

Signed-off-by: Leonard Lyubich <[email protected]>
@cthulhu-rider cthulhu-rider removed the blocked Can't be done because of something label Aug 14, 2025
@cthulhu-rider cthulhu-rider merged commit c6a9bed into master Aug 14, 2025
18 of 19 checks passed
@cthulhu-rider cthulhu-rider deleted the ec-place branch August 14, 2025 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants