Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19258 )
Change subject: KUDU-3371 [util] add NextOf method for ObjectIdGenerator ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/19258/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19258/2//COMMIT_MSG@9 PS2, Line 9: use > nit: using Done http://gerrit.cloudera.org:8080/#/c/19258/2//COMMIT_MSG@10 PS2, Line 10: usage cases > nit: use cases Done http://gerrit.cloudera.org:8080/#/c/19258/2//COMMIT_MSG@12 PS2, Line 12: plus 1, to act as the upper exclusion key when scan. > ... plus 1: that's to provide an exclusive upper bound when scanning Done http://gerrit.cloudera.org:8080/#/c/19258/2/src/kudu/util/oid_generator.h File src/kudu/util/oid_generator.h: http://gerrit.cloudera.org:8080/#/c/19258/2/src/kudu/util/oid_generator.h@47 PS2, Line 47: static size_t IdLength() { return 32; } > Add 'constexpr' to the signature to allow for compile-time optimization? I want to take advantage of the "Prefix Seek"[1] feature of rocksdb, for higher performance when scanning keys with the same prefix. The prefix is the container's id, I will use both IdLength() and NextOf() in next patches. 1. https://github.com/facebook/rocksdb/wiki/Prefix-Seek http://gerrit.cloudera.org:8080/#/c/19258/2/src/kudu/util/oid_generator.cc File src/kudu/util/oid_generator.cc: http://gerrit.cloudera.org:8080/#/c/19258/2/src/kudu/util/oid_generator.cc@68 PS2, Line 68: string next(id); : next[next.size() - 1] += 1; : return next; > Just curious why to choose this approach instead of appending an extra char The key in rocksdb is designed to combine contianer's id and block's id, and joined by a dot ("."), for example: abcd.123 abcd.124 abce.123 abce.124 As mentioned above, I want to take advantage of the "Prefix Seek", when the upper is the successor key of scanned prefix, it can be taken. Of course, any appending extra character can be taken advantage of it too, but '.' is a part of the real key, so it cannot be used since Scan["abcd", "abcd.") result empty. I checked the ASCII order, '.' is 46, '0' is 48, 'A' is 65, 'a' is 97, any character which is greater than '.' can be used too. But if we change the combined key format in rocksdb, for example remove or change the join character '.', the upper key must be changed too, if we forget to do that, there may be issues(some key can not be scanned out, or etc). So I choose this approach to make it to be definite correct. -- To view, visit http://gerrit.cloudera.org:8080/19258 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I293baa379e8d64adc4ff93acdb0713aae2731c7d Gerrit-Change-Number: 19258 Gerrit-PatchSet: 2 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Sun, 04 Dec 2022 12:39:17 +0000 Gerrit-HasComments: Yes
