Skip to content

Conversation

@End-rey
Copy link
Contributor

@End-rey End-rey commented Jul 14, 2025

Closes #3439.

@codecov
Copy link

codecov bot commented Jul 14, 2025

Codecov Report

❌ Patch coverage is 44.93671% with 87 lines in your changes missing coverage. Please review.
✅ Project coverage is 25.70%. Comparing base (adf5444) to head (16adc2d).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
pkg/services/object/get/util.go 14.58% 41 Missing ⚠️
pkg/services/object/get/assembly_v2.go 0.00% 30 Missing ⚠️
pkg/services/object/get/exec.go 73.68% 6 Missing and 4 partials ⚠️
pkg/services/object/get/assemble.go 83.78% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3466      +/-   ##
==========================================
+ Coverage   25.66%   25.70%   +0.03%     
==========================================
  Files         659      659              
  Lines       49373    49399      +26     
==========================================
+ Hits        12673    12698      +25     
+ Misses      35694    35691       -3     
- Partials     1006     1010       +4     

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

@End-rey End-rey force-pushed the 3439-stream-get-replies branch from 57aec91 to 44bc0fb Compare July 21, 2025 13:47
@End-rey End-rey requested a review from roman-khimov July 22, 2025 10:58
@End-rey End-rey force-pushed the 3439-stream-get-replies branch 4 times, most recently from bee7074 to b0e25b8 Compare August 15, 2025 14:17
@End-rey End-rey mentioned this pull request Aug 15, 2025
@End-rey End-rey marked this pull request as draft August 15, 2025 14:59
@End-rey End-rey force-pushed the 3439-stream-get-replies branch from b0e25b8 to a19cf2c Compare August 18, 2025 19:51
@End-rey End-rey marked this pull request as ready for review August 19, 2025 08:52
Copy link
Contributor

@cthulhu-rider cthulhu-rider left a comment

Choose a reason for hiding this comment

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

viewed partially, lets discuss these ones first

@End-rey End-rey force-pushed the 3439-stream-get-replies branch from a19cf2c to 5a26b99 Compare August 22, 2025 10:42
@End-rey End-rey requested a review from cthulhu-rider August 22, 2025 13:54
@End-rey End-rey force-pushed the 3439-stream-get-replies branch 2 times, most recently from 937f63d to 8b14bf0 Compare August 27, 2025 15:27
Copy link
Contributor

@cthulhu-rider cthulhu-rider left a comment

Choose a reason for hiding this comment

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

looks good overall

pls try to make more ergonomic diff in initFromChild, currently it has many unrelated changes

@End-rey End-rey force-pushed the 3439-stream-get-replies branch from 8b14bf0 to 12dacaf Compare August 29, 2025 13:31
Copy link
Contributor

@cthulhu-rider cthulhu-rider left a comment

Choose a reason for hiding this comment

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

@End-rey good!

lets cmp performance of 64K and 256K buffers. If there is no growth or it is insignificant, it makes sense to leave 64

and conflicts ofc

@End-rey End-rey force-pushed the 3439-stream-get-replies branch from 12dacaf to 22207c0 Compare August 29, 2025 14:47
@End-rey
Copy link
Contributor Author

End-rey commented Aug 29, 2025

@cthulhu-rider I think the last changes is assemble V1 split, which almost does not change the logic. And resolve conflicts in util.go.

@cthulhu-rider
Copy link
Contributor

tests fail

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xecbba3]

goroutine 302 [running]:
github.com/nspcc-dev/neofs-node/pkg/services/object/get.(*execCtx).writeObjectPayload(0xc000f9a360, 0x8?, {0x14d2490?, 0xc000046cb0})
	github.com/nspcc-dev/neofs-node/pkg/services/object/get/exec.go:334 +0xa3
github.com/nspcc-dev/neofs-node/pkg/services/object/get.(*execCtx).writeCollectedObject(0xc000f9a360)
	github.com/nspcc-dev/neofs-node/pkg/services/object/get/exec.go:377 +0x3b
github.com/nspcc-dev/neofs-node/pkg/services/object/get.(*execCtx).executeLocal(0xc000f9a360)
	github.com/nspcc-dev/neofs-node/pkg/services/object/get/local.go:29 +0xfb
github.com/nspcc-dev/neofs-node/pkg/services/object/get.(*execCtx).execute(0xc000f9a360)
	github.com/nspcc-dev/neofs-node/pkg/services/object/get/get.go:101 +0x3f
github.com/nspcc-dev/neofs-node/pkg/services/object/get.(*Service).get(0xc000282588, {0x14d9ed0, 0xc0004fa7b0}, {{0x14d10e8, 0xc00051cd50}, 0xc0004ba300, {{0xe8, 0x60, 0xda, 0xd3, ...}, ...}, ...}, ...)
	github.com/nspcc-dev/neofs-node/pkg/services/object/get/get.go:92 +0x165
github.com/nspcc-dev/neofs-node/pkg/services/object/get.(*Service).getRange(0xc000282588, {0x14d9ed0, 0xc0004fa7b0}, {{{0x14d10e8, 0xc00051cd50}, 0xc0004ba300, {{0xe8, 0x60, 0xda, 0xd3, ...}, ...}, ...}, ...}, ...)
	github.com/nspcc-dev/neofs-node/pkg/services/object/get/get.go:23 +0x159
github.com/nspcc-dev/neofs-node/pkg/services/object/get.(*Service).GetRange(...)
	github.com/nspcc-dev/neofs-node/pkg/services/object/get/get.go:19
main.(*objectSvc).GetRange(0x14dea30?, {0x14d9ed0?, 0xc0004fa7b0?}, {{{0x14d10e8, 0xc00051cd50}, 0xc0004ba300, {{0xe8, 0x60, 0xda, 0xd3, ...}, ...}, ...}, ...})
	github.com/nspcc-dev/neofs-node/cmd/neofs-node/object.go:94 +0x5f
github.com/nspcc-dev/neofs-node/pkg/services/object.(*Server).GetRange(0xc0002b5290, 0xc00053c280, {0x14e26f0, 0xc000046c10})
	github.com/nspcc-dev/neofs-node/pkg/services/object/server.go:1314 +0x44a
github.com/nspcc-dev/neofs-sdk-go/proto/object._ObjectService_GetRange_Handler({0x1250900, 0xc0002b5290}, {0x14debe0, 0xc000162000})
	github.com/nspcc-dev/[email protected]/proto/object/service_grpc.pb.go:779 +0x107
google.golang.org/grpc.(*Server).processStreamingRPC(0xc0002b3c00, {0x14d9ed0, 0xc0004fa660}, 0xc0004f4120, 0xc001509620, 0x1ee9ec0, 0x0)
	google.golang.org/[email protected]/server.go:1690 +0x126b
google.golang.org/grpc.(*Server).handleStream(0xc0002b3c00, {0x14dac80, 0xc00018f380}, 0xc0004f4120)
	google.golang.org/[email protected]/server.go:1814 +0xb69
google.golang.org/grpc.(*Server).serveStreams.func2.1()
	google.golang.org/[email protected]/server.go:1030 +0x7f
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 321
	google.golang.org/[email protected]/server.go:1041 +0x125

@End-rey End-rey force-pushed the 3439-stream-get-replies branch from 22207c0 to 0e825a3 Compare September 1, 2025 10:27
Copy link
Member

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

Please add some high-level statistics to 256K commit message (maybe to the first one as well), we need this to be kept in git log.

@End-rey End-rey force-pushed the 3439-stream-get-replies branch 2 times, most recently from 97e14e3 to a45a657 Compare September 5, 2025 08:44
@roman-khimov
Copy link
Member

Hmm, some problem with UTs after rebase.

@roman-khimov roman-khimov mentioned this pull request Sep 5, 2025
@End-rey End-rey force-pushed the 3439-stream-get-replies branch 2 times, most recently from 40bec1c to 16adc2d Compare September 5, 2025 09:59
Network API always had a concept of payload stream, FSTree can also provide it
now after #3431. So use `GetStream` in object service to enable memory-efficient
object retrieval without loading large payloads into memory.

Closes #3439.

Signed-off-by: Andrey Butusov <[email protected]>
```
Test=get---------Size=4096k---------Thr=15
base: 397 obj/s
64K:  448 obj/s
256K: 539 obj/s
```

Signed-off-by: Andrey Butusov <[email protected]>
Use `apistatus.ErrObjectOutOfRange` instead of `new(apistatus.ObjectOutOfRange)`

Signed-off-by: Andrey Butusov <[email protected]>
@roman-khimov roman-khimov merged commit dd83657 into master Sep 5, 2025
22 checks passed
@roman-khimov roman-khimov deleted the 3439-stream-get-replies branch September 5, 2025 15:55
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.

Stream GET replies

5 participants