[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/20725 )
Change subject: KUDU-3527 Fix block manager test on 64k filesystems
..
Patch Set 7:
(9 comments)
Overall looks good to me, but I think this patch needs a bit of clean-up at
least easier comprehension later on when it's time to enable the test.
http://gerrit.cloudera.org:8080/#/c/20725/7/src/kudu/fs/block_manager-test.cc
File src/kudu/fs/block_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/20725/7/src/kudu/fs/block_manager-test.cc@928
PS7, Line 928: const int kNumTries = 3;
: const int kNumBlockTries = 500;
: const int kNumAppends = 4;
nit: could these be constexpr?
http://gerrit.cloudera.org:8080/#/c/20725/7/src/kudu/fs/block_manager-test.cc@937
PS7, Line 937: With non-zero crash_on_eio
nit: With crash_on_eio enabled
http://gerrit.cloudera.org:8080/#/c/20725/7/src/kudu/fs/block_manager-test.cc@939
PS7, Line 939: It is not the
: // scope of this test to test this case.
This is looks quite confusing given what this test scenario is doing. Instead
of this, maybe it's better to add a comment at line 926 to explain why this
test is here and why it's added as a disabled one?
Overall, please update this block comment, clarifying on the following points:
* the 'what' part: what this test scenario is doing
* the 'why' part: why this test scenario is here and why it is added as a
disabled one?
* the 'when' part: when this test scenario should be enabled, if ever?
You might consider splitting it in multiple piece, placing some of them at line
926, prefacing the scenario.
http://gerrit.cloudera.org:8080/#/c/20725/7/src/kudu/fs/block_manager-test.cc@942
PS7, Line 942: arm
nit ARM
http://gerrit.cloudera.org:8080/#/c/20725/7/src/kudu/fs/block_manager-test.cc@942
PS7, Line 942: like the 64k option of rhel 8.8
What are those exactly?
http://gerrit.cloudera.org:8080/#/c/20725/7/src/kudu/fs/block_manager-test.cc@951
PS7, Line 951: If you set a lower value, the test becomes flaky. With 0.2 it
almost always fails.
This might become confusing later on when the root cause of KUDU-3528 is
addressed: it's not very likely that this comment will be fixed once the
DISABLED_ prefix is removed.
Maybe, just mention what exactly happens here, not resorting to 'flaky' and
'test failure' terms? I'd think of explaining that the scenario ends up with
non-empty .data file and no .meta file much less often if FLAGS_env_inject_eio
set to a value less than 0.2.
http://gerrit.cloudera.org:8080/#/c/20725/7/src/kudu/fs/block_manager-test.cc@958
PS7, Line 958: the result
nit: the result block ID
http://gerrit.cloudera.org:8080/#/c/20725/7/src/kudu/fs/block_manager-test.cc@982
PS7, Line 982: if (s.ok()) {
: InsertOrDie(&ids, id);
: num_created++;
: }
If this is just a way to overcome injected IO errors, why not to set
FLAGS_env_inject_eio = 0 in the scope while calling create_a_block()?
Otherwise, please add a comment explaining why it's important to have IO error
injections enabled when creating blocks.
http://gerrit.cloudera.org:8080/#/c/20725/7/src/kudu/fs/block_manager-test.cc@999
PS7, Line 999: if (it != ids.end()) {
: it++;
: }
nit for here and elsewhere: it would be great if you preserved more of the
original comments from the TestMetadataOkayDespiteFailure scenario that are
still helpful to the reader to understand what's the intention behind some of
these actions.
--
To view, visit http://gerrit.cloudera.org:8080/20725
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20
Gerrit-Change-Number: 20725
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Martonka
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Ashwani Raina
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Zoltan Martonka
Gerrit-Comment-Date: Fri, 19 Jan 2024 23:55:43 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20725 ) Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/20725/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20725/7//COMMIT_MSG@10 PS7, Line 10: fs_block_size What's "fs_block_size"? Is it indeed the filesystem's block size (i.e. what 'stat -f' reports) or this is about the IO block size (a.k.a. optimal block size for I/O, or what 'stat' reports)? It would great to have more clarity here in the commit message at least since there is a lot of confusion around this topic already, and bringing in some more isn't a good idea, not at all. Same for the comments in the code, etc. On my Graviton3 VM, the actual FS block size for an XFS filesystem is 4K, but the optimal IO block size is 64K: [root@aserbin-rh88-graviton3 ~]# stat -f .bashrc File: ".bashrc" ID: 10303 Namelen: 255 Type: xfs Block size: 4096 Fundamental block size: 4096 Blocks: Total: 52243707 Free: 9130378Available: 9130378 Inodes: Total: 73555248 Free: 73076679 [root@aserbin-rh88-graviton3 ~]# stat .bashrc File: .bashrc Size: 307 Blocks: 8 IO Block: 65536 regular file Device: 10303h/66307d Inode: 1223994 Links: 1 Access: (0644/-rw-r--r--) Uid: (0/root) Gid: (0/root) Access: 2024-01-19 11:03:05.043364091 -0800 Modify: 2023-10-06 16:47:49.797395518 -0700 Change: 2023-10-06 16:47:49.797395518 -0700 Birth: 2023-10-06 16:47:49.797395518 -0700 -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 7 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Zoltan Martonka Gerrit-Comment-Date: Fri, 19 Jan 2024 19:17:58 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Hello Mahesh Reddy, Alexey Serbin, Ashwani Raina, Kudu Jenkins, Abhishek Chennaka, Wang Xixu, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20725 to look at the new patch set (#7). Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. KUDU-3527 Fix block manager test on 64k filesystems BlockManagerTest.TestMetadataOkayDespiteFailure might fail on systems where fs_block_size=64k. Currently tablets fail to load if one metadata is missing but there is still a non-empty ".data" file. If FLAGS_env_inject_eio is not zero, then there is a chance that, when we delete the container file, we only delete the ".meta", but leave the ".data" file. In the current test on systems with fs_block_size=4k deletion never occurs. Changing to kNumAppends=64 will cause the test to randomly fail on x86 systems too, although only with a 2-3% chance (at least on my ubuntu20 machine). The short term solution is to make sure no container becomes full. A jira was created: KUDU-3528 and a test was added: DISABLED_TestReloadBlockManagerDespiteFailure for the original issue. Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 --- M src/kudu/fs/block_manager-test.cc 1 file changed, 98 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/20725/7 -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 7 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Zoltan Martonka
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Zoltan Martonka has posted comments on this change. ( http://gerrit.cloudera.org:8080/20725 ) Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG@7 PS5, Line 7: Fix block manager test on 64k filesystems > Just for the record and to make things simpler here, you could make use of I commented on other Gerrit ( https://gerrit.cloudera.org/#/c/20680/), please continue there. -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Zoltan Martonka Gerrit-Comment-Date: Thu, 14 Dec 2023 14:04:19 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/20725 ) Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG@7 PS5, Line 7: Fix block manager test on 64k filesystems > For f_bsize=64k, I do the following. Just for the record and to make things simpler here, you could make use of following commands to create a loop device and run mkfs.xfs on it instead of adding an additional disk: (works on Ubuntu, check for equivalent flags/commands on other distributions) 1. "losetup -f" -> Gives next available unused loop device. 2. "dd if=/dev/zero of=/mnt/file bs=4096 count=262144" - create 1GB file 3. Use the loop device from the #1 output and attach to the file -> "losetup /dev/ /mnt/file" 4. Now, you can run mkfs.xfs > "mkfs.xfs -b size=65536 /dev/ On a side note, when you say 64K kernel, you mean 64K kernel page size - right? With that, you should be able to mount the filesystem with 64K block size. I had asked about hole punching question on https://gerrit.cloudera.org/#/c/20680/ but I guess it was not fully tried out. What is the error you got when you tried to start Kudu? Basically, I am interested in knowing whether it is our code or underlying kernel code that throws error. KUDU-3523, however, is using overlayfs (abstract layer like unionfs) where Kudu is up and running. It is worth exploring how come Kudu is able to start there and not in this environment. -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Zoltan Martonka Gerrit-Comment-Date: Thu, 14 Dec 2023 13:54:13 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Zoltan Martonka has posted comments on this change. ( http://gerrit.cloudera.org:8080/20725 ) Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG@7 PS5, Line 7: Fix block manager test on 64k filesystems > Could you give more detail about the test on system where f_bsize=64k? For f_bsize=64k, I do the following. Start an rhel 8.8 with 64k kernel. Mount an additional disk and format it: mkfs.xfs -b /dsize=65536 -f /dev/ (to my best understanding you should prefer using such formating with 64k kernel or use the 4k kernel version of rhel arm). mount it and put kudu data dir on it (export TEST_TMPDIR=/mnt/disk2/kudu_test) you will have f_bsize=64k and st_blksize=64k. Kudu wont start because “Status CheckHolePunch(…)” in dir_util.cc has hardcoded values (also needs fixing). I took a deeper look at https://gerrit.cloudera.org/#/c/20680/. I think I finally start to understand the problem. Your fix is necessary (f_bsize is indeed what hole punch/ preallocation uses). However it is just one of multiple fixes that are needed. I suggest we continue the discussion in 20680 For the fix in this request: Fixing the “f_bsize > 4k || st_blksize > 4k” problem wont fix BlockManagerTest.TestMetadataOkayDespiteFailure. It will still fail for f_bsize=64k disk (as it does even on x86_64 with minor changes). I thing we should mere this fix, as it is just accidentally correlated with the f_bsize issues. However if you guys would like to keep it open until other issues are fixed, and we can test it on f_bsize=64k system, then we can leave it open. -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Zoltan Martonka Gerrit-Comment-Date: Thu, 14 Dec 2023 12:52:32 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/20725 ) Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. Patch Set 6: (1 comment) > Patch Set 5: > > (1 comment) http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG@7 PS5, Line 7: Fix block manager test on 64k filesystems > Yes I did, but I don't fully understand the purpose of it. Could you give more detail about the test on system where f_bsize=64k? I meets the same problem. You can use a shell command: 'stat -f ' to get f_bsize and use command: 'stat ' to get st_blksize. Please give the detail on issue KUDU-3527. Thanks. -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 6 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Zoltan Martonka Gerrit-Comment-Date: Thu, 07 Dec 2023 10:01:29 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Zoltan Martonka has posted comments on this change. ( http://gerrit.cloudera.org:8080/20725 ) Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. Patch Set 6: I created a different ticket for the original issue: https://issues.apache.org/jira/browse/KUDU-3528 Also provided slight modifications to make the test fail on normal x86_64 machines. I would like to deliver this now, and fix the original issue later (because it is a long task, and has nothing to do with arm or the original purpose of this test). -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 6 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Zoltan Martonka Gerrit-Comment-Date: Fri, 01 Dec 2023 10:00:09 + Gerrit-HasComments: No
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Zoltan Martonka has posted comments on this change. ( http://gerrit.cloudera.org:8080/20725 ) Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/20725/5/src/kudu/fs/block_manager-test.cc File src/kudu/fs/block_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/20725/5/src/kudu/fs/block_manager-test.cc@936 PS5, Line 936: FLAGS_log_container_max_size = 512 > I am not quite clear on why we need to increase container max size. It woul I tried to give an other explanation. Also instead of increasing the container size, so deletion of the log block file itself never happens, I realised it is easier to disable the dead container deletion: FLAGS_log_block_manager_delete_dead_container = false; -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Zoltan Martonka Gerrit-Comment-Date: Thu, 30 Nov 2023 16:21:48 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Hello Mahesh Reddy, Ashwani Raina, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20725 to look at the new patch set (#6). Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. KUDU-3527 Fix block manager test on 64k filesystems BlockManagerTest.TestMetadataOkayDespiteFailure might fail on systems where fs_block_size=64k. Currently tablets fail to load if one metadata is missing but there is still a non-empty ".data" file. If FLAGS_env_inject_eio is not zero, then there is a chance that, when we delete the container file, we only delete the ".meta", but leave the ".data" file. In the current test on systems with fs_block_size=4k deletion never occurs. Changing to kNumAppends=64 will cause the test to randomly fail on x86 systems too, although only with a 2-3% chance (at least on my ubuntu20 machine). The short term solution is to make sure no container becomes full. Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 --- M src/kudu/fs/block_manager-test.cc 1 file changed, 14 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/20725/6 -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 6 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Zoltan Martonka
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Zoltan Martonka has posted comments on this change. ( http://gerrit.cloudera.org:8080/20725 ) Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG@7 PS5, Line 7: Fix block manager test on 64k filesystems > Have you looked at this patch -> https://gerrit.cloudera.org/#/c/20680/ Yes I did, but I don't fully understand the purpose of it. In our current cloudcat rhel 8.8 image f_bsize is 4k, so it would fix the test. But if in some system f_bsize=64k too, then the test would still fail. Basically if GetBlockSize() return >=64k, then the test has a chance to fail. -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Zoltan Martonka Gerrit-Comment-Date: Thu, 30 Nov 2023 13:49:16 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/20725 ) Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG@7 PS5, Line 7: Fix block manager test on 64k filesystems Have you looked at this patch -> https://gerrit.cloudera.org/#/c/20680/ Take a look if you haven't, to see if there is any correlation. http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG@13 PS5, Line 13: chanche chance http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG@18 PS5, Line 18: altough although http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG@18 PS5, Line 18: chanche chance http://gerrit.cloudera.org:8080/#/c/20725/5//COMMIT_MSG@25 PS5, Line 25: sort short http://gerrit.cloudera.org:8080/#/c/20725/5/src/kudu/fs/block_manager-test.cc File src/kudu/fs/block_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/20725/5/src/kudu/fs/block_manager-test.cc@936 PS5, Line 936: FLAGS_log_container_max_size = 512 I am not quite clear on why we need to increase container max size. It would be great if you could add some more details here. -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Thu, 30 Nov 2023 12:29:33 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20725 ) Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/20725/5/src/kudu/fs/block_manager-test.cc File src/kudu/fs/block_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/20725/5/src/kudu/fs/block_manager-test.cc@940 PS5, Line 940: chanche nit: chance -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Thu, 30 Nov 2023 03:50:43 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20725 to look at the new patch set (#5). Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. KUDU-3527 Fix block manager test on 64k filesystems BlockManagerTest.TestMetadataOkayDespiteFailure might fail on systems where fs_block_size=64k. Currently tablets fail to load if one metadata is missing but there is still a non-empty ".data" file. If FLAGS_env_inject_eio is not zero, then there is a chanche that, when we delete the container file, we only delete the ".meta", but leave the ".data" file. In the current test on systems with fs_block_size=4k deletion never occurs. Changing to kNumAppends=64 will cause the test to randomly fail on x86 systems too, altough only with a 2-3% chanche (at least on my ubuntu20 machine). The real solution would be to make tablet loading more error resistant, there are plans for that, but it is not trivial: https://issues.apache.org/jira/browse/KUDU-3458 The sort term solution is to make sure no container becomes full. Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 --- M src/kudu/fs/block_manager-test.cc 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/20725/5 -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20725 to look at the new patch set (#4). Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. KUDU-3527 Fix block manager test on 64k filesystems BlockManagerTest.TestMetadataOkayDespiteFailure might fail on systems where fs_block_size=64k. Currently tablets fail to load if one metadata is missing but there is still a non-empty ".data" file. If FLAGS_env_inject_eio is not zero, then there is a chanche that, when we delete the container file, we only delete the ".meta", but leave the ".data" file. In the current test on systems with fs_block_size=4k deletion never occurs. Changing to kNumAppends=64 will cause the test to randomly fail on x86 systems too, altough only with a 2-3% chanche (at least on my ubuntu20 machine). The real solution would be to make tablet loading more error resistant, there are plans for that, but it is not trivial: https://issues.apache.org/jira/browse/KUDU-3458 The sort term solution is to make sure no container becomes full. Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 --- M src/kudu/fs/block_manager-test.cc 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/20725/4 -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20725 to look at the new patch set (#3). Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. KUDU-3527 Fix block manager test on 64k filesystems BlockManagerTest.TestMetadataOkayDespiteFailure might fail on systems where fs_block_size=64k. Currently tablets fail to load if one metadata is missing but there is still a non-empty ".data" file. If FLAGS_env_inject_eio is not zero, then there is a chanche that, when we delete the container file, we only delete the ".meta", but leave the ".data" file. In the current test on systems with fs_block_size=4k deletion never occurs. Changing to kNumAppends=64 will cause the test to randomly fail on x86 systems too, altough only with a 2-3% chanche (at least on my ubuntu20 machine). The real solution would be to make tablet loading more error resistant, there are plans for that, but it is not trivial: https://issues.apache.org/jira/browse/KUDU-3458 The sort term solution is to make sure no container becomes full. Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 --- M src/kudu/fs/block_manager-test.cc 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/20725/3 -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20725 to look at the new patch set (#2). Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. KUDU-3527 Fix block manager test on 64k filesystems BlockManagerTest.TestMetadataOkayDespiteFailure might fail on systems where fs_block_size=64k. Currently tablets fail to load if one metadata is missing but there is still a non-empty ".data" file. If FLAGS_env_inject_eio is not zero, then there is a chanche that, when we delete the container file, we only delete the ".meta", but leave the ".data" file. In the current test on systems with fs_block_size=4k deletion never occurs. Changing to kNumAppends=64 will cause the test to randomly fail on x86 systems too, altough only with a 2-3% chanche (at least on my ubuntu20 machine). The real solution would be to make tablet loading more error resistant, there are plans for that, but it is not trivial: https://issues.apache.org/jira/browse/KUDU-3458 The sort term solution is to make sure no container becomes full. Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 --- M src/kudu/fs/block_manager-test.cc 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/20725/2 -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Martonka Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3527 Fix block manager test on 64k filesystems
Zoltan Martonka has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20725 Change subject: KUDU-3527 Fix block manager test on 64k filesystems .. KUDU-3527 Fix block manager test on 64k filesystems BlockManagerTest.TestMetadataOkayDespiteFailure might fail on systems where fs_block_size=64k. Currently tablets fail to load if one metadata is missing but there is still a non-empty ".data" file. If FLAGS_env_inject_eio is not zero, then there is a chanche that, when we delete the container file, we only delete the ".meta", but leave the ".data" file. In the current test on systems with fs_block_size=4k deletion never occurs. Changing to kNumAppends=64 will cause the test to randomly fail on x86 systems too, altough only with a 2-3% chanche (at least on my ubuntu20 machine). The real solution would be to make tablet loading more error resistant, there are plans for that, but it is not trivial: https://issues.apache.org/jira/browse/KUDU-3458 The sort term solution is to make sure no container becomes full. Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 --- M src/kudu/fs/block_manager-test.cc 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/20725/1 -- To view, visit http://gerrit.cloudera.org:8080/20725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7e325bde502b7d7f39dd17fa84cb7eb42a3d7c20 Gerrit-Change-Number: 20725 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Martonka
