Multinet dialer support #521

Merged
alexvanin merged 1 commit from alexvanin/frostfs-s3-gw:feature/multinet into master 2024-11-02 14:21:44 +00:00
Owner
Adoption of https://git.frostfs.info/TrueCloudLab/frostfs-node/pulls/1422
alexvanin added 2 commits 2024-10-21 12:30:07 +00:00
Code is taken from frostfs-node#1422
Author: Dmitrii Stepanov (dstepanov-yadro)

Signed-off-by: Alex Vanin <a.vanin@yadro.com>
[#xxx] Use source dialer for gRPC connection to storage
Some checks failed
/ DCO (pull_request) Failing after 1m26s
/ Vulncheck (pull_request) Successful in 1m32s
/ Builds (pull_request) Successful in 1m28s
/ Lint (pull_request) Failing after 1m50s
/ Tests (pull_request) Successful in 1m41s
c101be6af3
Signed-off-by: Alex Vanin <a.vanin@yadro.com>
alexvanin force-pushed feature/multinet from c101be6af3 to 20035065ff 2024-10-21 12:52:15 +00:00 Compare
alexvanin force-pushed feature/multinet from 20035065ff to 4e20704a1e 2024-10-21 12:55:26 +00:00 Compare
alexvanin requested review from dstepanov-yadro 2024-10-21 12:55:35 +00:00
alexvanin added 1 commit 2024-10-22 13:16:55 +00:00
[#521] Add documentation for multinet settings
Some checks failed
/ DCO (pull_request) Successful in 1m3s
/ Vulncheck (pull_request) Successful in 1m35s
/ Builds (pull_request) Successful in 1m4s
/ Lint (pull_request) Failing after 1m40s
/ Tests (pull_request) Successful in 1m30s
6b0559f917
Signed-off-by: Alex Vanin <a.vanin@yadro.com>
alexvanin force-pushed feature/multinet from 6b0559f917 to a24965daf6 2024-10-22 13:18:40 +00:00 Compare
alexvanin requested review from dkirillov 2024-10-22 13:41:40 +00:00
alexvanin requested review from pogpp 2024-10-22 13:41:41 +00:00
alexvanin requested review from mbiryukova 2024-10-22 13:41:41 +00:00
alexvanin requested review from r.loginov 2024-10-22 13:41:41 +00:00
alexvanin requested review from nzinkevich 2024-10-22 13:41:41 +00:00
alexvanin changed title from WIP: Multinet dialer support to Multinet dialer support 2024-10-22 13:41:50 +00:00
alexvanin force-pushed feature/multinet from a24965daf6 to bea1e2d818 2024-10-22 14:13:11 +00:00 Compare
dkirillov reviewed 2024-10-23 11:29:35 +00:00
@ -0,0 +8,4 @@
"golang.org/x/sys/unix"
)
func newDefaulDialer() net.Dialer {
Member

Typo: defaul vs default

Typo: `defaul` vs `default`
Author
Owner

Fixed.

Fixed.
dkirillov marked this conversation as resolved
@ -0,0 +16,4 @@
// with OS defaults for keepalive time and interval, use a net.Dialer that sets
// the KeepAlive field to a negative value, and sets the SO_KEEPALIVE socket
// option to true from the Control field. For a concrete example of how to do
// this, see internal.NetDialerWithTCPKeepalive().
Member

It seems we also need add some todo
830135e6c5/internal/tcp_keepalive_unix.go (L33C15-L33C56)
?

It seems we also need add some todo https://github.com/grpc/grpc-go/blob/830135e6c5a351abf75f0c9cfdf978e5df8daeba/internal/tcp_keepalive_unix.go#L33C15-L33C56 ?
Author
Owner

Updated.

Updated.
dkirillov marked this conversation as resolved
@ -0,0 +25,4 @@
}
func (s *DialerSource) build(c Config) error {
if c.Enabled {
Member

Can we invert condition to reduce if block length?

Can we invert condition to reduce `if` block length?
Author
Owner

I don't mind but I've tried to keep code the same with storage node, so it will be easier to fix things here and there. What you prefer more, having similar code or fixing code style?

I don't mind but I've tried to keep code the same with storage node, so it will be easier to fix things here and there. What you prefer more, having similar code or fixing code style?
Member

I don't have a strong opinion

I don't have a strong opinion
Author
Owner

I've decided to keep code the same with frostfs-node.

I've decided to keep code the same with frostfs-node.
dkirillov marked this conversation as resolved
@ -0,0 +60,4 @@
// NetContextDialer returns net.DialContext dial function.
// Returns nil if multinet disabled.
func (s *DialerSource) NetContextDialer() func(context.Context, string, string) (net.Conn, error) {
Member

Currently this is unused. Do we really need add this right now?

Currently this is unused. Do we really need add this right now?
Author
Owner

Sure, missed that. I've already removed some unused code from storage implementation such as DialContextTCP

Sure, missed that. I've already removed some unused code from storage implementation such as [DialContextTCP](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65a4320c7539f4915e799f652475c739b7ad58b7/internal/net/dialer.go#L16)
Author
Owner

Fixed.

Fixed.
dkirillov marked this conversation as resolved
alexvanin force-pushed feature/multinet from bea1e2d818 to 53fdf52178 2024-10-28 13:39:53 +00:00 Compare
dkirillov approved these changes 2024-10-28 13:47:42 +00:00
dkirillov left a comment
Member

LGTM

LGTM
mbiryukova approved these changes 2024-10-28 15:32:48 +00:00
alexvanin force-pushed feature/multinet from 53fdf52178 to 8bc19725ba 2024-10-29 14:40:44 +00:00 Compare
dstepanov-yadro approved these changes 2024-10-29 14:58:29 +00:00
pogpp approved these changes 2024-10-30 06:59:25 +00:00
alexvanin merged commit 8bc19725ba into master 2024-10-31 08:40:36 +00:00
alexvanin deleted branch feature/multinet 2024-10-31 08:40:40 +00:00
alexvanin added this to the v0.31.0 milestone 2024-11-20 12:13:32 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
5 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-s3-gw#521
No description provided.