[kudu-CR] KUDU-3523 Use f bsize as Kudu block size instead of st blksize
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/20680 ) Change subject: KUDU-3523 Use f_bsize as Kudu block size instead of st_blksize .. Patch Set 4: > Patch Set 4: > > > Do you have any real problem running kudu? > > For me Kudu seems to kind of handle f_bsize != st_blksize kind of. > > So it can run an such a disk (maybe not by optimal performance). > > So backward compatibility is an issue, since there can be a Kudu > > running on such a system. which means only this flag solution can > > work. > > Also some of these consistently tests fail on x86_64 to if you > > make an ifs disk with 1k blocks. > > > > I think using f_bsize instead of st_blksize might actually be the > > better way on 64k rhel if the disk is 4k. > > > > So I support the addition of these flags. > > However I think we should extend/fix the tests to test both false > > and true values of the flag. > > *xfs disk Thanks for your comments. Kudu can run on a 64k rhel system without fix, only some unit tests will fail. -- To view, visit http://gerrit.cloudera.org:8080/20680 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I04343caf5fe377a4fa9b57e6e450307e6b995928 Gerrit-Change-Number: 20680 Gerrit-PatchSet: 4 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Zoltan Martonka Gerrit-Comment-Date: Wed, 17 Jan 2024 06:06:05 + Gerrit-HasComments: No
[kudu-CR] [net] DiagnosticSocket wrapper for sock diag API
Hello Yingchun Lai, Attila Bukor, Yifan Zhang, Kudu Jenkins, Abhishek Chennaka, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20892 to look at the new patch set (#3). Change subject: [net] DiagnosticSocket wrapper for sock_diag API .. [net] DiagnosticSocket wrapper for sock_diag API This patch introduces DiagnosticSocket wrapper for sock_diag() netlink subsystem [1][2]. The primary intention behind DiagnosticSocket is to fetch information on the RX queue size for a listening IPv4 TCP socket. A follow-up patch will use this new facility to populate a new server-level metric: that's to track connection request backlog for a Kudu server's RPC socket (a.k.a. listened socket backlog, pending connections queue, etc.). Since netlink is a Linux-specific API/subsystem, the DiagnosticSocket API is available only on Linux, correspondingly. This patch includes a few unit tests to provide basic coverage for the newly introduced functionality. [1] https://man7.org/linux/man-pages/man7/sock_diag.7.html [2] https://man7.org/linux/man-pages/man7/netlink.7.html Change-Id: I4a4f37ca4b0df8ca543ec383d89766cbf1b1e526 --- M src/kudu/util/CMakeLists.txt A src/kudu/util/net/diagnostic_socket-test.cc A src/kudu/util/net/diagnostic_socket.cc A src/kudu/util/net/diagnostic_socket.h 4 files changed, 619 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/20892/3 -- To view, visit http://gerrit.cloudera.org:8080/20892 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4a4f37ca4b0df8ca543ec383d89766cbf1b1e526 Gerrit-Change-Number: 20892 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai
[kudu-CR] [rpc] introduce rpc listened socket rx queue size metric
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20908 Change subject: [rpc] introduce rpc_listened_socket_rx_queue_size metric .. [rpc] introduce rpc_listened_socket_rx_queue_size metric This patch introduces a new 'rpc_listened_socket_rx_queue_size' histogram metric for AcceptorPool. The metric allows for tracking the size of the listened RPC socket's RX queue. The new metric shows meaningful numbers only on Linux since it's based on the DiagnosticSocket, where the latter is implemented only on Linux as of now. I also added basic tests scenarios to cover the newly introduced functionality. Change-Id: I83580659bac39d9171f1ee0d0e88676ed0d50b99 --- M src/kudu/rpc/acceptor_pool.cc M src/kudu/rpc/acceptor_pool.h M src/kudu/rpc/rpc-test-base.h M src/kudu/rpc/rpc-test.cc M src/kudu/util/CMakeLists.txt M src/kudu/util/net/diagnostic_socket.cc M src/kudu/util/net/diagnostic_socket.h 7 files changed, 224 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/08/20908/1 -- To view, visit http://gerrit.cloudera.org:8080/20908 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I83580659bac39d9171f1ee0d0e88676ed0d50b99 Gerrit-Change-Number: 20908 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] [net] DiagnosticSocket wrapper for sock diag API
Hello Yingchun Lai, Attila Bukor, Yifan Zhang, Kudu Jenkins, Abhishek Chennaka, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20892 to look at the new patch set (#2). Change subject: [net] DiagnosticSocket wrapper for sock_diag API .. [net] DiagnosticSocket wrapper for sock_diag API This patch introduces DiagnosticSocket wrapper for sock_diag() netlink subsystem [1][2]. The primary intention behind DiagnosticSocket is to fetch information on the RX queue size for a listening IPv4 TCP socket. A follow-up patch will use this new facility to populate a new server-level metric: that's to track the backlog of connection requests to Kudu servers' RPC sockets. Since netlink is a Linux-specific API/subsystem, the DiagnosticSocket API is available only on Linux, correspondingly. This patch includes a few unit tests to provide basic coverage for the newly introduced functionality. [1] https://man7.org/linux/man-pages/man7/sock_diag.7.html [2] https://man7.org/linux/man-pages/man7/netlink.7.html Change-Id: I4a4f37ca4b0df8ca543ec383d89766cbf1b1e526 --- M src/kudu/util/CMakeLists.txt A src/kudu/util/net/diagnostic_socket-test.cc A src/kudu/util/net/diagnostic_socket.cc A src/kudu/util/net/diagnostic_socket.h 4 files changed, 619 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/20892/2 -- To view, visit http://gerrit.cloudera.org:8080/20892 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4a4f37ca4b0df8ca543ec383d89766cbf1b1e526 Gerrit-Change-Number: 20892 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai
[kudu-CR] [net] DiagnosticSocket wrapper for sock diag API
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20892 ) Change subject: [net] DiagnosticSocket wrapper for sock_diag API .. Patch Set 1: (5 comments) Thank you for feedback! http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/diagnostic_socket.h File src/kudu/util/net/diagnostic_socket.h: http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/diagnostic_socket.h@17 PS1, Line 17: #pragma once > nit: leave a blank line Done http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/diagnostic_socket.cc File src/kudu/util/net/diagnostic_socket.cc: http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/diagnostic_socket.cc@75 PS1, Line 75: socket > nit:: It would be better to add "::" prefixes for the system functions, oth Done http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/diagnostic_socket.cc@115 PS1, Line 115: RETURN_NOT_OK(ReceiveResponse(&result)); > I'm just curious why not judge the "result" size like the below? With this method, it's possible to have multiple items in the result because of providing corresponding parameters. You can see how it's used in the DiagnosticSocketTest.SimplePattern's code. http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/socket-test.cc File src/kudu/util/net/socket-test.cc: http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/socket-test.cc@247 PS1, Line 247: s.Release(); > Is it necessary to call Release manually? Not exactly necessary. I removed this. http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/socket.cc File src/kudu/util/net/socket.cc: http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/socket.cc@245 PS1, Line 245: false > Is it possible the 'fd' is an already listened fd? Yep, that's a good point. It seems these changes were made in a mechanical way. And frankly, these code had a smell (faint one, though). After some consideration, I decided to remove this 'IsListened' and rely on GetPeerAddress() instead -- that seems to be cleaner. -- To view, visit http://gerrit.cloudera.org:8080/20892 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a4f37ca4b0df8ca543ec383d89766cbf1b1e526 Gerrit-Change-Number: 20892 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Comment-Date: Tue, 16 Jan 2024 17:27:00 + Gerrit-HasComments: Yes
[kudu-CR] [log block manager-test] Improve random selection
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20899 ) Change subject: [log_block_manager-test] Improve random selection .. Patch Set 5: > Patch Set 5: > > (1 comment) Besides this error, everything else lgtm. -- To view, visit http://gerrit.cloudera.org:8080/20899 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f73f0572319784b07845b02d15a7d3e6f31abce Gerrit-Change-Number: 20899 Gerrit-PatchSet: 5 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Zoltan Martonka Gerrit-Reviewer: Ádám Bakai Gerrit-Comment-Date: Tue, 16 Jan 2024 15:46:02 + Gerrit-HasComments: No
[kudu-CR] KUDU-3523 Use f bsize as Kudu block size instead of st blksize
Zoltan Martonka has posted comments on this change. ( http://gerrit.cloudera.org:8080/20680 ) Change subject: KUDU-3523 Use f_bsize as Kudu block size instead of st_blksize .. Patch Set 4: > Do you have any real problem running kudu? > For me Kudu seems to kind of handle f_bsize != st_blksize kind of. > So it can run an such a disk (maybe not by optimal performance). > So backward compatibility is an issue, since there can be a Kudu > running on such a system. which means only this flag solution can > work. > Also some of these consistently tests fail on x86_64 to if you > make an ifs disk with 1k blocks. > > I think using f_bsize instead of st_blksize might actually be the > better way on 64k rhel if the disk is 4k. > > So I support the addition of these flags. > However I think we should extend/fix the tests to test both false > and true values of the flag. *xfs disk -- To view, visit http://gerrit.cloudera.org:8080/20680 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I04343caf5fe377a4fa9b57e6e450307e6b995928 Gerrit-Change-Number: 20680 Gerrit-PatchSet: 4 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Zoltan Martonka Gerrit-Comment-Date: Tue, 16 Jan 2024 14:58:00 + Gerrit-HasComments: No
[kudu-CR] KUDU-3523 Use f bsize as Kudu block size instead of st blksize
Zoltan Martonka has posted comments on this change. ( http://gerrit.cloudera.org:8080/20680 ) Change subject: KUDU-3523 Use f_bsize as Kudu block size instead of st_blksize .. Patch Set 4: Do you have any real problem running kudu? For me Kudu seems to kind of handle f_bsize != st_blksize kind of. So it can run an such a disk (maybe not by optimal performance). So backward compatibility is an issue, since there can be a Kudu running on such a system. which means only this flag solution can work. Also some of these consistently tests fail on x86_64 to if you make an ifs disk with 1k blocks. I think using f_bsize instead of st_blksize might actually be the better way on 64k rhel if the disk is 4k. So I support the addition of these flags. However I think we should extend/fix the tests to test both false and true values of the flag. -- To view, visit http://gerrit.cloudera.org:8080/20680 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I04343caf5fe377a4fa9b57e6e450307e6b995928 Gerrit-Change-Number: 20680 Gerrit-PatchSet: 4 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Reviewer: Zoltan Martonka Gerrit-Comment-Date: Tue, 16 Jan 2024 14:56:41 + Gerrit-HasComments: No
[kudu-CR] [net] DiagnosticSocket wrapper for sock diag API
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/20892 ) Change subject: [net] DiagnosticSocket wrapper for sock_diag API .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/diagnostic_socket.h File src/kudu/util/net/diagnostic_socket.h: http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/diagnostic_socket.h@17 PS1, Line 17: #pragma once nit: leave a blank line http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/diagnostic_socket.cc File src/kudu/util/net/diagnostic_socket.cc: http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/diagnostic_socket.cc@75 PS1, Line 75: socket nit:: It would be better to add "::" prefixes for the system functions, other placces are the same. http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/diagnostic_socket.cc@115 PS1, Line 115: RETURN_NOT_OK(ReceiveResponse(&result)); I'm just curious why not judge the "result" size like the below? http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/socket-test.cc File src/kudu/util/net/socket-test.cc: http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/socket-test.cc@247 PS1, Line 247: s.Release(); Is it necessary to call Release manually? http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/socket.cc File src/kudu/util/net/socket.cc: http://gerrit.cloudera.org:8080/#/c/20892/1/src/kudu/util/net/socket.cc@245 PS1, Line 245: false Is it possible the 'fd' is an already listened fd? -- To view, visit http://gerrit.cloudera.org:8080/20892 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a4f37ca4b0df8ca543ec383d89766cbf1b1e526 Gerrit-Change-Number: 20892 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai Gerrit-Comment-Date: Tue, 16 Jan 2024 12:54:54 + Gerrit-HasComments: Yes
[kudu-CR] [log block manager-test] Improve random selection
Hello Tidy Bot, Zoltan Martonka, Kudu Jenkins, Wang Xixu, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20899 to look at the new patch set (#5). Change subject: [log_block_manager-test] Improve random selection .. [log_block_manager-test] Improve random selection The previous solution to remove random blocks was not good, because it was possible that way less blocks are removed than expected because it is possible that rand()%100 returns 0 or 99 every time and that means in extreme cases zero or all the elements are deleted. The new approach removes exactly the same number of blocks every time but randomizes which blocks are removed and the order of removal is randomized, too. Change-Id: I9f73f0572319784b07845b02d15a7d3e6f31abce --- M src/kudu/fs/log_block_manager-test.cc 1 file changed, 5 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/20899/5 -- To view, visit http://gerrit.cloudera.org:8080/20899 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f73f0572319784b07845b02d15a7d3e6f31abce Gerrit-Change-Number: 20899 Gerrit-PatchSet: 5 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Zoltan Martonka Gerrit-Reviewer: Ádám Bakai
[kudu-CR] [log block manager-test] Improve random selection
Ádám Bakai has posted comments on this change. (
http://gerrit.cloudera.org:8080/20899 )
Change subject: [log_block_manager-test] Improve random selection
..
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/20899/4/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/20899/4/src/kudu/fs/log_block_manager-test.cc@1296
PS4, Line 1296: auto rng = std::default_random_engine{};
> Did you consider seting a random seed?
Done
http://gerrit.cloudera.org:8080/#/c/20899/4/src/kudu/fs/log_block_manager-test.cc@1298
PS4, Line 1298: ids.erase(ids.begin(), ids.begin() +
static_cast(kLiveBlockRatio * kNumBlocks));
> ids is a local variable in this 17 lines lambda. The current way seems kind
I'm not sure either if it makes easier to understand.
http://gerrit.cloudera.org:8080/#/c/20899/4/src/kudu/fs/log_block_manager-test.cc@1298
PS4, Line 1298: ids.erase(ids.begin(), ids.begin() +
static_cast(kLiveBlockRatio * kNumBlocks));
> Is it better to use another variable to store the to-delete ids, for exampl
Done
--
To view, visit http://gerrit.cloudera.org:8080/20899
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f73f0572319784b07845b02d15a7d3e6f31abce
Gerrit-Change-Number: 20899
Gerrit-PatchSet: 4
Gerrit-Owner: Ádám Bakai
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Zoltan Martonka
Gerrit-Reviewer: Ádám Bakai
Gerrit-Comment-Date: Tue, 16 Jan 2024 11:42:48 +
Gerrit-HasComments: Yes
[kudu-CR] Fix CheckHolePunch for bigger than 4k blocks.
Hello Ashwani Raina, Kudu Jenkins, Wang Xixu, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20894 to look at the new patch set (#5). Change subject: Fix CheckHolePunch for bigger than 4k blocks. .. Fix CheckHolePunch for bigger than 4k blocks. With the new 64k ARM core, you can use filesystems with up to 64k block size. Currently Kudu does not start if the data is on a filesystem with larger than 4k blocks. Problem: CheckHolePunch has hard-coded 4k block size instead of using the get env->GetBlockSize. Kudu seems to be running fine on a 64k fs after this fix. More info: https://kudu.apache.org/docs/troubleshooting.html#req_hole_punching Change-Id: Ib4af73f0aa25db674fe0a34355cecd27a0c68417 --- M src/kudu/fs/dir_util.cc 1 file changed, 8 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/20894/5 -- To view, visit http://gerrit.cloudera.org:8080/20894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib4af73f0aa25db674fe0a34355cecd27a0c68417 Gerrit-Change-Number: 20894 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Zoltan Martonka
[kudu-CR] Fix CheckHolePunch for bigger than 4k blocks.
Zoltan Martonka has posted comments on this change. ( http://gerrit.cloudera.org:8080/20894 ) Change subject: Fix CheckHolePunch for bigger than 4k blocks. .. Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/20894/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20894/2//COMMIT_MSG@12 PS2, Line 12: > Maybe you could add more details on what fails in this test if underlying f I just wanted to give a test, so if some1 wants to try it out, he does not have to run a whole server. I changed the commit message, so It wont confuse anyone. It is not a unit test fix. This is required for kudu to start http://gerrit.cloudera.org:8080/#/c/20894/1/src/kudu/fs/dir_util.cc File src/kudu/fs/dir_util.cc: http://gerrit.cloudera.org:8080/#/c/20894/1/src/kudu/fs/dir_util.cc@78 PS1, Line 78: ETURN_NOT_OK(env->GetBlockSize(filename, > There are some failed tests, please check it. Thank you. http://gerrit.cloudera.org:8080/#/c/20894/2/src/kudu/fs/dir_util.cc File src/kudu/fs/dir_util.cc: http://gerrit.cloudera.org:8080/#/c/20894/2/src/kudu/fs/dir_util.cc@a68 PS2, Line 68: > Just wondering, if we keep offset at 4K which is unaligned for a 64K file-s We use ioctl in case of xfs. (see Status PunchHole in env_posix.cc) It does not report any error. Currently we fail with error message in line 96. (since file will be 64k). If we reserve a multiple of 64k, but make an unaligned or not long enough hole punch, then we fail with the message in line 105. -- To view, visit http://gerrit.cloudera.org:8080/20894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib4af73f0aa25db674fe0a34355cecd27a0c68417 Gerrit-Change-Number: 20894 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Zoltan Martonka Gerrit-Comment-Date: Tue, 16 Jan 2024 11:00:51 + Gerrit-HasComments: Yes
[kudu-CR] Fix CheckHolePunch for bigger than 4k blocks.
Hello Ashwani Raina, Kudu Jenkins, Wang Xixu, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20894 to look at the new patch set (#4). Change subject: Fix CheckHolePunch for bigger than 4k blocks. .. Fix CheckHolePunch for bigger than 4k blocks. With the new 64k ARM core, you can use filesystems with up to 64k block size. Currently Kudu does not start if the data is on a filesystem with larger than 4k blocks. Problem: CheckHolePunch has hard-coded 4k block size instead of using the get env->GetBlockSize. Kudu seems to be running fine on a 64k fs after this fix. More info: https://kudu.apache.org/docs/troubleshooting.html#req_hole_punching Change-Id: Ib4af73f0aa25db674fe0a34355cecd27a0c68417 --- M src/kudu/fs/dir_util.cc 1 file changed, 8 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/20894/4 -- To view, visit http://gerrit.cloudera.org:8080/20894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib4af73f0aa25db674fe0a34355cecd27a0c68417 Gerrit-Change-Number: 20894 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
[kudu-CR] Fix CheckHolePunch for bigger than 4k blocks.
Hello Ashwani Raina, Kudu Jenkins, Wang Xixu, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20894 to look at the new patch set (#3). Change subject: Fix CheckHolePunch for bigger than 4k blocks. .. Fix CheckHolePunch for bigger than 4k blocks. With the new 64k ARM core, you can use filesystems with up to 64k block size. Currently Kudu does not start if the data is on a filesystem with larger than 4k blocks. Problem: CheckHolePunch has hard-coded 4k block size instead of using the get env->GetBlockSize. Kudu seems to be running fine on a 64k fs after this fix. Change-Id: Ib4af73f0aa25db674fe0a34355cecd27a0c68417 --- M src/kudu/fs/dir_util.cc 1 file changed, 8 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/20894/3 -- To view, visit http://gerrit.cloudera.org:8080/20894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib4af73f0aa25db674fe0a34355cecd27a0c68417 Gerrit-Change-Number: 20894 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
[kudu-CR] Fix CheckHolePunch for bigger than 4k blocks.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/20894 ) Change subject: Fix CheckHolePunch for bigger than 4k blocks. .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/20894/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20894/2//COMMIT_MSG@12 PS2, Line 12: ContainerPreallocationTest Maybe you could add more details on what fails in this test if underlying filesystem has 64K block size. http://gerrit.cloudera.org:8080/#/c/20894/2/src/kudu/fs/dir_util.cc File src/kudu/fs/dir_util.cc: http://gerrit.cloudera.org:8080/#/c/20894/2/src/kudu/fs/dir_util.cc@a68 PS2, Line 68: Just wondering, if we keep offset at 4K which is unaligned for a 64K file-system, what is the current behaviour? Does fallocate fail for unaligned offset and length? -- To view, visit http://gerrit.cloudera.org:8080/20894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib4af73f0aa25db674fe0a34355cecd27a0c68417 Gerrit-Change-Number: 20894 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Comment-Date: Tue, 16 Jan 2024 10:04:34 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Hello Marton Greber, Alexey Serbin, Yifan Zhang, Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20715 to look at the new patch set (#28). Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java client. .. KUDU-3526 [java] Scanner should bind with a tserver in java client. For now, scan requests sent by the java client might fail with scanner not found error. It is because that scanner does not bind with the tserver with which it first communicates. The code in method scanNextRows of java client is still trying to search for the tserver by the locations and selection policy. So if the leader is changed and the next scan request is sent to the new leader, the tserver will respond with the "scanner not found" exception. We should bind the scanner to the tserver, similar to how it is done in the C++ client. Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java 4 files changed, 116 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/15/20715/28 -- To view, visit http://gerrit.cloudera.org:8080/20715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2 Gerrit-Change-Number: 20715 Gerrit-PatchSet: 28 Gerrit-Owner: Song Jiacheng Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Song Jiacheng Gerrit-Reviewer: Yifan Zhang
[kudu-CR] KUDU-3526 [java] Scanner should bind with a tserver in java client.
Song Jiacheng has posted comments on this change. (
http://gerrit.cloudera.org:8080/20715 )
Change subject: KUDU-3526 [java] Scanner should bind with a tserver in java
client.
..
Patch Set 28:
(2 comments)
> Patch Set 27:
>
> (2 comments)
Thanks for the view!
http://gerrit.cloudera.org:8080/#/c/20715/27/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
File
java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/20715/27/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@291
PS27, Line 291: bound w
> nit: bound
Done
http://gerrit.cloudera.org:8080/#/c/20715/27/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
File java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java:
http://gerrit.cloudera.org:8080/#/c/20715/27/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java@312
PS27, Line 312: return tabletServers.get(uuid);
> nit: Should we also handle the case where uuid is null?
For now it returns null if uuid is null, which I think it should return. The
client will try to find the target tablet server via cache or sending request.
final ServerInfo info = tablet.getTabletServerByUuid(scanner.getTsUUID());
if (info == null) {
return delayedSendRpcToTablet(nextRequest, new
RecoverableException(Status.RemoteError(
String.format("No information on servers hosting tablet %s, will
retry later",
tablet.getTabletId();
}
--
To view, visit http://gerrit.cloudera.org:8080/20715
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9cf65f4215e99198dd41b43d14e50c8c23b8a9b2
Gerrit-Change-Number: 20715
Gerrit-PatchSet: 28
Gerrit-Owner: Song Jiacheng
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber
Gerrit-Reviewer: Song Jiacheng
Gerrit-Reviewer: Yifan Zhang
Gerrit-Comment-Date: Tue, 16 Jan 2024 08:16:55 +
Gerrit-HasComments: Yes
