[kudu-CR] [Java] KUDU-3498 Scanner keeps alive in periodically
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20761 ) Change subject: [Java] KUDU-3498 Scanner keeps alive in periodically .. Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/20761/7/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/20761/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@970 PS7, Line 970:* @param scanner the scanner to keep alive. nit: Do we have this parameter for the method? http://gerrit.cloudera.org:8080/#/c/20761/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java: http://gerrit.cloudera.org:8080/#/c/20761/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@434 PS7, Line 434: scanner.nextRows(); nit: We should also verify that we can read all rows inserted without 'scanner not found' error occurs. http://gerrit.cloudera.org:8080/#/c/20761/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@474 PS7, Line 474: /* :* Test keeping a scanner alive periodically beyond scanner ttl. :*/ nit: Update this comment. -- To view, visit http://gerrit.cloudera.org:8080/20761 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I50648e987b72aead472a20ff4336e3e7f23d8e06 Gerrit-Change-Number: 20761 Gerrit-PatchSet: 7 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Comment-Date: Tue, 30 Jan 2024 08:26:38 + Gerrit-HasComments: Yes
[kudu-CR] [WIP][catalog manager] Skip eviction during bootstrapping
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20958 to look at the new patch set (#4). Change subject: [WIP][catalog_manager] Skip eviction during bootstrapping .. [WIP][catalog_manager] Skip eviction during bootstrapping If the leader replica shuts down and returns to the quorum, it will take some time to bootstrap. During this time, it is possible to send a full tablet report it still reckons itself as the leader and since it is bootstrapping, the overall health is unknown. This can cause trouble as it there is a check that the leader must be always healthy. This issue is solved by checking if the tablet is bootstrapping and if it is then eviction check is skipped. There is no need for new test because this change fixes ToolTest::TestRecreateCMeta flakiness and verifies the correct behaviour indirectly. Change-Id: I0cbbe6b77d99d167fc58692047072f86d3d963cf --- M src/kudu/master/catalog_manager.cc 1 file changed, 4 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/20958/4 -- To view, visit http://gerrit.cloudera.org:8080/20958 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0cbbe6b77d99d167fc58692047072f86d3d963cf Gerrit-Change-Number: 20958 Gerrit-PatchSet: 4 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [log block manager] Write lock for deletion
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20901 to look at the new patch set (#8). Change subject: [log_block_manager] Write lock for deletion .. [log_block_manager] Write lock for deletion Both removing blocks and compacting metadata manipulates the live_blocks_ value and file content. This way it can happen that on thread A live_blocks_ is updated, on thread B metadata is compacted and live_blocks_ value is overwritten, then thread A writes deletion records in the metadata file. This way it creates inconsistency between file content and live_blocks_ value, which leads to incorrect behaviour to skip compaction later. This fix makes sure that CompactMetadata and deletion can not overlap each other. No new test is needed because it fixes LogBlockManagerTest:: TestContainerBlockLimitingByMetadataSizeWithCompaction flakiness. Change-Id: I73712a5fd8eccf23621f8abda3e99ebdf10a769f --- M src/kudu/fs/log_block_manager.cc 1 file changed, 2 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/20901/8 -- To view, visit http://gerrit.cloudera.org:8080/20901 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I73712a5fd8eccf23621f8abda3e99ebdf10a769f Gerrit-Change-Number: 20901 Gerrit-PatchSet: 8 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [Java] KUDU-3498 Scanner keeps alive in periodically
Hello Yifan Zhang, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20761 to look at the new patch set (#8). Change subject: [Java] KUDU-3498 Scanner keeps alive in periodically .. [Java] KUDU-3498 Scanner keeps alive in periodically Kudu caches the scanner in the tablet server for continuing reading. It will be expired if the idle time is over the defined scanner ttl time. Sometimes the client reads a batch of data, if the data is every large, it takes a long time to handle it. Then the client reads the next batch using the same scanner, the scanner will be expired even if it has sent a keep alive request. This patch adds support for keeping a scanner alive periodically. It uses a timer to send keep alive requests background. So, it will never be expired when the scanner is in using. Change-Id: I50648e987b72aead472a20ff4336e3e7f23d8e06 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java 3 files changed, 205 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/20761/8 -- To view, visit http://gerrit.cloudera.org:8080/20761 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I50648e987b72aead472a20ff4336e3e7f23d8e06 Gerrit-Change-Number: 20761 Gerrit-PatchSet: 8 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang
[kudu-CR] [Java] KUDU-3498 Scanner keeps alive in periodically
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/20761 ) Change subject: [Java] KUDU-3498 Scanner keeps alive in periodically .. Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/20761/7/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/20761/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@970 PS7, Line 970:* @param keepAliveIntervalMS the interval of sending keep-alive requests. > nit: Do we have this parameter for the method? Done http://gerrit.cloudera.org:8080/#/c/20761/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java: http://gerrit.cloudera.org:8080/#/c/20761/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@434 PS7, Line 434: row_count += scanner.nextRows().getNumRows(); > nit: We should also verify that we can read all rows inserted without 'scan Yes, it should verify that we can read all rows inserted. And it is no need to check error 'scanner not found'. Because it will throw an exception if 'scanner not found' error occurs and the test will fail. http://gerrit.cloudera.org:8080/#/c/20761/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@474 PS7, Line 474: /* :* Test stoping the keep-alive timer. :*/ > nit: Update this comment. Done -- To view, visit http://gerrit.cloudera.org:8080/20761 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I50648e987b72aead472a20ff4336e3e7f23d8e06 Gerrit-Change-Number: 20761 Gerrit-PatchSet: 8 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang Gerrit-Comment-Date: Wed, 31 Jan 2024 03:18:21 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3371 [fs] Use RocksDB to store LBM metadata
Yingchun Lai has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18569 ) Change subject: KUDU-3371 [fs] Use RocksDB to store LBM metadata .. KUDU-3371 [fs] Use RocksDB to store LBM metadata Since the LogBlockContainerNativeMeta stores block records sequentially in the metadata file, the live blocks maybe in a very low ratio, so it may cause serious disk space amplification and long bootstrap times. This patch introduces a new class LogBlockContainerRdbMeta which uses RocksDB to store LBM metadata, a new item will be Put() into RocksDB when a new block is created in LBM, and the item will be Delete() from RocksDB when the block is removed from LBM. Data in RocksDB can be maintained by RocksDB itself, i.e. deleted items will be GCed so it's not needed to rewrite the metadata as how we do in LogBlockContainerNativeMeta. The implementation also reuses most logic of the base class LogBlockContainer, the main difference with LogBlockContainerNativeMeta is that LogBlockContainerRdbMeta stores block records metadata in RocksDB rather than in a native file. The main implementation of interfaces from the base class including: a. Create a container Data file is created similar to LogBlockContainerNativeMeta, but the metadata part is stored in RocksDB with keys constructed as ".", and values are the same to the records stored in metadata file of LogBlockContainerNativeMeta. b. Open a container Similar to LogBlockContainerNativeMeta, and it's not needed to check the metadata part, because it has been checked when loading containers during the bootstrap phase. c. Destroy a container If the container is dead (full and no live blocks), remove the data file, and clean up metadata part, by deleting all the keys prefixed by "". d. Load a container (by ProcessRecords()) Iterate the RocksDB in the key range [, ), because dead blocks have been deleted directly, thus only live block records will be populated, we can use them as LogBlockContainerNativeMeta. e. Create blocks in a container Put() serialized BlockRecordPB records into RocksDB, keys are constructed the same to the above. f. Remove blocks from a container Construct the keys same to the above, Delete() them from RocksDB in batch. This patch contains the following changes: - Adds a new block manager type named 'logr', it uses RocksDB to store LBM metadata. The new block manager is enabled by setting --block_manager=logr. - Related tests add new parameterized value to test the case of "--block_manager=logr". It's optional to use RocksDB, we can use the former LBM as before, we will introduce more tools to convert data between the two implementations in the future. The optimization is obvious as shown in JIRA KUDU-3371, it shows that the time spent to re-open tablet server's metadata when 99.99% of all the records removed reduced about 9.5 times when using LogBlockContainerRdbMeta instead of LogBlockContainerNativeMeta. Change-Id: Ie72f6914eb5653a9c034766c6cd3741a8340711f Reviewed-on: http://gerrit.cloudera.org:8080/18569 Tested-by: Alexey Serbin Reviewed-by: Alexey Serbin --- M src/kudu/benchmarks/CMakeLists.txt M src/kudu/client/CMakeLists.txt M src/kudu/consensus/CMakeLists.txt M src/kudu/fs/CMakeLists.txt M src/kudu/fs/block_manager-stress-test.cc M src/kudu/fs/block_manager-test.cc M src/kudu/fs/block_manager.h M src/kudu/fs/data_dirs.cc M src/kudu/fs/dir_manager.cc M src/kudu/fs/dir_manager.h M src/kudu/fs/dir_util.cc M src/kudu/fs/file_block_manager.h M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/fs_manager.h M src/kudu/fs/fs_report.cc M src/kudu/fs/fs_report.h M src/kudu/fs/log_block_manager-test-util.cc M src/kudu/fs/log_block_manager-test-util.h M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/fs/log_block_manager.h M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/dense_node-itest.cc M src/kudu/integration-tests/ts_recovery-itest.cc M src/kudu/server/CMakeLists.txt M src/kudu/tablet/compaction-test.cc M src/kudu/tools/CMakeLists.txt M src/kudu/tools/kudu-tool-test.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/util/CMakeLists.txt M thirdparty/build-definitions.sh 32 files changed, 1,782 insertions(+), 185 deletions(-) Approvals: Alexey Serbin: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/18569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie72f6914eb5653a9c034766c6cd3741a8340711f Gerrit-Change-Number: 18569 Gerrit-PatchSet: 74 Gerrit-Owner: Yingchun Lai Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: KeDeng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan
[kudu-CR] KUDU-613: Scan Resistant Caching
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/20607 )
Change subject: KUDU-613: Scan Resistant Caching
..
Patch Set 8:
(3 comments)
I took a quick look.
My request is to split this changelist into multiple pieces, at least separate
out the following:
* refactoring on cache.{h,cc} and nvm_cache.{h,cc} that preclude introducing
the SLRU cache
* introducing the SLRU cache
* introducing block cache metrics
Thank you!
http://gerrit.cloudera.org:8080/#/c/20607/8/src/kudu/util/block_cache_metrics.h
File src/kudu/util/block_cache_metrics.h:
PS8:
Why is it necessary to update the block cache metrics in the same changelist
where implementing a new type of a cache?
Is it possible to introduce block cache metrics in a separate follow-up
changelist?
http://gerrit.cloudera.org:8080/#/c/20607/8/src/kudu/util/cache.h
File src/kudu/util/cache.h:
http://gerrit.cloudera.org:8080/#/c/20607/8/src/kudu/util/cache.h@67
PS8, Line 67: // Recency list cache implementations (FIFO, LRU, etc.)
Is it possible to separate out this and other refactoring (i.e. moving
duplicated code from nvm_cache.{h,cc} and cache.{h,cc}) into a separate
changelist?
http://gerrit.cloudera.org:8080/#/c/20607/8/src/kudu/util/slru_cache.h
File src/kudu/util/slru_cache.h:
http://gerrit.cloudera.org:8080/#/c/20607/8/src/kudu/util/slru_cache.h@50
PS8, Line 50: class HandleDeleter {
:public:
: explicit HandleDeleter(SLRUCache* c)
: : c_(c) {
: }
:
: void operator()(Handle* h) const {
: if (h != nullptr) {
: c_->Release(h);
: }
: }
:
: SLRUCache* cache() const {
: return c_;
: }
:
:private:
: SLRUCache* c_;
: };
nit: maybe, it's time to templatize this class and re-use it throughout various
types of caches in util?
--
To view, visit http://gerrit.cloudera.org:8080/20607
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I45531534a2049dd38c002f4dc7982df9fd46e0bb
Gerrit-Change-Number: 20607
Gerrit-PatchSet: 8
Gerrit-Owner: Mahesh Reddy
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 31 Jan 2024 04:44:16 +
Gerrit-HasComments: Yes
