You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Ashwani Raina (Code Review)" <ge...@cloudera.org> on 2025/05/26 07:34:09 UTC
[kudu-CR] [socket] No need for static check of SO REUSEPORT macro
Ashwani Raina has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22945
Change subject: [socket] No need for static check of SO_REUSEPORT macro
......................................................................
[socket] No need for static check of SO_REUSEPORT macro
SO_REUSEPORT socket option support was added quite a while back.
For linux, support was added to kernel version 3.9.
- RHEL: Available since RHEL 6.5, which uses version 2.6.32 of the Linux kernel.
- CentOS: Support present in release version 7 with default kernel version 3.10.0.
- Ubuntu: Support present in release version 18.04 with default kernel version 4.15.
macOS: Support present in v11 (Big Sur), verified on Xcode version 11.4.
Change-Id: I477d47251a45a57e4179d8affd2273cb25768383
---
M src/kudu/util/net/socket.cc
1 file changed, 4 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/22945/1
--
To view, visit http://gerrit.cloudera.org:8080/22945
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I477d47251a45a57e4179d8affd2273cb25768383
Gerrit-Change-Number: 22945
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
[kudu-CR] [socket] No need for static check of SO REUSEPORT macro
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22945 )
Change subject: [socket] No need for static check of SO_REUSEPORT macro
......................................................................
Patch Set 1:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/1395/
--
To view, visit http://gerrit.cloudera.org:8080/22945
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477d47251a45a57e4179d8affd2273cb25768383
Gerrit-Change-Number: 22945
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 26 May 2025 07:34:16 +0000
Gerrit-HasComments: No
[kudu-CR] [socket] No need for static check of SO REUSEPORT macro
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22945 )
Change subject: [socket] No need for static check of SO_REUSEPORT macro
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/22945
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477d47251a45a57e4179d8affd2273cb25768383
Gerrit-Change-Number: 22945
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 28 May 2025 19:25:04 +0000
Gerrit-HasComments: No
[kudu-CR] [socket] No need for static check of SO REUSEPORT macro
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/22945 )
Change subject: [socket] No need for static check of SO_REUSEPORT macro
......................................................................
[socket] No need for static check of SO_REUSEPORT macro
SO_REUSEPORT socket option support was added quite a while back.
For Linux, support was added to kernel version 3.9.
- RHEL: Available since RHEL 6.5, which uses version 2.6.32 of the
Linux kernel.
- CentOS: Support present in release version 7 with default Linux
kernel version 3.10.0.
- Ubuntu: Support present in release version 18.04 with default
Linux kernel version 4.15.
For macOS, support is present in v11 (Big Sur), verified on Xcode
version 11.4.
Change-Id: I477d47251a45a57e4179d8affd2273cb25768383
Reviewed-on: http://gerrit.cloudera.org:8080/22945
Reviewed-by: Alexey Serbin <al...@apache.org>
Tested-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/util/net/socket.cc
1 file changed, 4 insertions(+), 8 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/22945
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I477d47251a45a57e4179d8affd2273cb25768383
Gerrit-Change-Number: 22945
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [socket] No need for static check of SO REUSEPORT macro
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22945
to look at the new patch set (#2).
Change subject: [socket] No need for static check of SO_REUSEPORT macro
......................................................................
[socket] No need for static check of SO_REUSEPORT macro
SO_REUSEPORT socket option support was added quite a while back.
For Linux, support was added to kernel version 3.9.
- RHEL: Available since RHEL 6.5, which uses version 2.6.32 of the
Linux kernel.
- CentOS: Support present in release version 7 with default Linux
kernel version 3.10.0.
- Ubuntu: Support present in release version 18.04 with default
Linux kernel version 4.15.
For macOS, support is present in v11 (Big Sur), verified on Xcode
version 11.4.
Change-Id: I477d47251a45a57e4179d8affd2273cb25768383
---
M src/kudu/util/net/socket.cc
1 file changed, 4 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/22945/2
--
To view, visit http://gerrit.cloudera.org:8080/22945
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I477d47251a45a57e4179d8affd2273cb25768383
Gerrit-Change-Number: 22945
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [socket] No need for static check of SO REUSEPORT macro
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/22945 )
Change subject: [socket] No need for static check of SO_REUSEPORT macro
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/22945/1//COMMIT_MSG
Commit Message:
PS1:
> formatting nit: could you reformat the description to follow the guidelines
Done.
I wonder if we can hook commit message rules to git that does all these checks.
There is one link that does describes steps to achieve something similar: https://dev.to/mbarzeev/a-git-hook-for-commit-messages-validation-no-husky-just-js-1hni
It would be worth trying though, could make life easier.
http://gerrit.cloudera.org:8080/#/c/22945/1//COMMIT_MSG@10
PS1, Line 10:
> nit: Linux
Done
--
To view, visit http://gerrit.cloudera.org:8080/22945
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477d47251a45a57e4179d8affd2273cb25768383
Gerrit-Change-Number: 22945
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 28 May 2025 04:57:13 +0000
Gerrit-HasComments: Yes
[kudu-CR] [socket] No need for static check of SO REUSEPORT macro
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22945 )
Change subject: [socket] No need for static check of SO_REUSEPORT macro
......................................................................
Patch Set 1: Code-Review+1
(2 comments)
LGTM, just a few nits for the commit description
http://gerrit.cloudera.org:8080/#/c/22945/1//COMMIT_MSG
Commit Message:
PS1:
formatting nit: could you reformat the description to follow the guidelines outlined at https://kudu.apache.org/docs/contributing.html as 'generic git commit guidelines and good practices', at particular https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project#_commit_guidelines
Thank you!
http://gerrit.cloudera.org:8080/#/c/22945/1//COMMIT_MSG@10
PS1, Line 10: linux
nit: Linux
--
To view, visit http://gerrit.cloudera.org:8080/22945
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477d47251a45a57e4179d8affd2273cb25768383
Gerrit-Change-Number: 22945
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 27 May 2025 16:27:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] [socket] No need for static check of SO REUSEPORT macro
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22945 )
Change subject: [socket] No need for static check of SO_REUSEPORT macro
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22945/1//COMMIT_MSG
Commit Message:
PS1:
> Done.
Right: as of now, people can introduce any helpful automation as git hooks in their workspaces. In my case, I configured my editor to take care of that (i.e. break lines where needed) when I'm writing commit messages.
As for gerrit itself, one can see the ruler (dotted vertical line) on the right, so it's easy to spot those nits when reviewing patches. Probably, there is some level of automation to automate vetting of patches in gerrit based on that formatting criteria, but I haven't explored that.
However, it shouldn't be a hard rule to reject a commit with lines over 72 characters in description. Sometimes it's necessary to include a stack trace or a snippet from logs into the description as-is or with minimal changes.
It might be beneficial to add instructions of installing formatting hook helpers into the Kudu upstream docs, similar to the instructions of installing the Gerrit commit-msg hook. However, it's not a must-have (compared with Gerrit commit-msg hook) but rather a recommendation.
Feel free to explore this and continue discussion at #kudu-general Slack channel and corresponding mailing lists, if necessary.
--
To view, visit http://gerrit.cloudera.org:8080/22945
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477d47251a45a57e4179d8affd2273cb25768383
Gerrit-Change-Number: 22945
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 28 May 2025 19:24:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] [socket] No need for static check of SO REUSEPORT macro
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/22945 )
Change subject: [socket] No need for static check of SO_REUSEPORT macro
......................................................................
Patch Set 3:
> (1 comment)
I am using gerrit's new UI which probably doesn't have that dotted vertical line.
--
To view, visit http://gerrit.cloudera.org:8080/22945
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477d47251a45a57e4179d8affd2273cb25768383
Gerrit-Change-Number: 22945
Gerrit-PatchSet: 3
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 29 May 2025 11:30:05 +0000
Gerrit-HasComments: No
[kudu-CR] [socket] No need for static check of SO REUSEPORT macro
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22945 )
Change subject: [socket] No need for static check of SO_REUSEPORT macro
......................................................................
Patch Set 1: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/1395/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/22945
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I477d47251a45a57e4179d8affd2273cb25768383
Gerrit-Change-Number: 22945
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 26 May 2025 08:51:45 +0000
Gerrit-HasComments: No