Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20613 )

Change subject: Fix a unit test in aarch64 system
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20613/10/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:

http://gerrit.cloudera.org:8080/#/c/20613/10/src/kudu/fs/log_block_manager-test.cc@370
PS10, Line 370:   // For example, usually on x86 architecture it is 4k but on 
aarch64 some
              :   // linux distribution (eg. 4.19.90-23.30.v2101.ky10.aarch6) 
can use a 64k
              :   // block size kernel.
I think in this context it's not relevant what's the default block size in 
particular kernel or architecture, but rather what's the block size for the 
underlying file system, no?

Since the IO block size is now set according to the information read from the 
FS, I'd think we can remove the TODO comment above about 'Investigate this'.


http://gerrit.cloudera.org:8080/#/c/20613/10/src/kudu/fs/log_block_manager-test.cc@376
PS10, Line 376: 1024
Given the comment above, I'd think might be set to fs_block_size instead of 
hard-coded 1024, no?



--
To view, visit http://gerrit.cloudera.org:8080/20613
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide52a251b15b1af437d570c146beb0c30fed161b
Gerrit-Change-Number: 20613
Gerrit-PatchSet: 10
Gerrit-Owner: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Abhishek Chennaka <achenn...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <ale...@apache.org>
Gerrit-Reviewer: Attila Bukor <abu...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yifan Zhang <chinazhangyi...@163.com>
Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org>
Gerrit-Reviewer: Zoltan Martonka <zmarto...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Nov 2023 19:02:25 +0000
Gerrit-HasComments: Yes

Reply via email to