Node's get service refactor #1126

New issue

Open

opened 2025-12-28 17:21:53 +00:00 by sami · 2 comments

sami commented

2025-12-28 17:21:53 +00:00

Owner

Originally created by @carpawell on GitHub (Sep 14, 2023).

I'm always frustrated when I need to read and change the get object service in the Storage Node. IMO it has not been refactored for a too long time: some features were added and some hotfixes have been applied so now it always scares me when I need to add/change something in this service.

Describe the solution you'd like

Try to see which parts are in use, and which are outdated, to remove totally unused code. Rework forwarding mechanism (also think it may lead to some new thoughts about our access logic). Make it more clear what requests can spawn (and should they?) another requests (e.g. it is/was OK to make GETRANGE if the original request is GETRANGEHAS, the same was applied to GET if GETRANGE was declined). There are also some parasit small private structs and a lot of callback logic in the service (e.g. forwarding is a huge kludge IMO).

Describe alternatives you've considered

Keep it as is until some bug fix leads to another bug, or until some dev is not available to understand the service from scratch.

Additional context

#2557 could look better.

Originally created by @carpawell on GitHub (Sep 14, 2023). ## Is your feature request related to a problem? Please describe. I'm always frustrated when I need to read and change the get object service in the Storage Node. IMO it has not been refactored for a too long time: some features were added and some hotfixes have been applied so now it always scares me when I need to add/change something in this service. ## Describe the solution you'd like Try to see which parts are in use, and which are outdated, to remove totally unused code. Rework forwarding mechanism (also think it may lead to some new thoughts about our access logic). Make it more clear what requests can spawn (and should they?) another requests (e.g. it is/was OK to make `GETRANGE` if the original request is `GETRANGEHAS`, the same was applied to `GET` if `GETRANGE` was declined). There are also some parasit small private structs and a lot of callback logic in the service (e.g. [forwarding](https://github.com/nspcc-dev/neofs-node/blob/3749ce78dc175210e364dde4db42ffc448d1e89b/pkg/services/object/get/v2/util.go#L73) is a huge kludge IMO). ## Describe alternatives you've considered Keep it as is until some bug fix leads to another bug, or until some dev is not available to understand the service from scratch. ## Additional context #2557 could look better.

sami added the

enhancement

neofs-storage

blocked

labels

2025-12-28 17:21:53 +00:00

sami commented

2025-12-28 17:21:53 +00:00

Author

Owner

@cthulhu-rider commented on GitHub (Sep 14, 2023):

agree with the desire for a refactor, at the same time I would wait for very deep test coverage (i mean https://github.com/nspcc-dev/neofs-testcases) because this is a very sensitive part of the system

it's possible that the current coverage is sufficient, it needs to be revised

@cthulhu-rider commented on GitHub (Sep 14, 2023): agree with the desire for a refactor, at the same time I would wait for very deep test coverage (i mean https://github.com/nspcc-dev/neofs-testcases) because this is a very sensitive part of the system it's possible that the current coverage is sufficient, it needs to be revised

sami commented

2025-12-28 17:21:53 +00:00

Author

Owner

@carpawell commented on GitHub (Sep 25, 2023):

Agree. Do we have any issues in the tests repo? Do we have an understanding of the exact coverage that suits us?

@carpawell commented on GitHub (Sep 25, 2023): Agree. Do we have any issues in the tests repo? Do we have an understanding of the exact coverage that suits us?