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 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
requested review from dstepanov-yadro 2024-10-21 12:55:35 +00:00
alexvanin force-pushed feature/multinet from 6b0559f917 to a24965daf6 2024-10-22 13:18:40 +00:00 Compare
requested reviews from dkirillov, pogpp, mbiryukova, r.loginov, 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.