[kudu-CR] KUDU-1457 p1: krpc supports for IPv6
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/15996 ) Change subject: KUDU-1457 p1: krpc supports for IPv6 .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/15996/2/src/kudu/util/net/net_util.h File src/kudu/util/net/net_util.h: http://gerrit.cloudera.org:8080/#/c/15996/2/src/kudu/util/net/net_util.h@142 PS2, Line 142: Network(sa_family_t family, uint128_t addr, uint128_t netmask); > I think this Network class is only used for our whitelisting of which subne Thanks for your suggests, but let me clarify it if I understand correctly: ipv4 still uses the Network class, but ipv6 uses an uint64 instead? Or parent/child inheritance class: class Network --> class NetworkIPv4/class NetworkIPv6/ In addition, should we use another flag to indicate trusted ipv6 subnets? http://gerrit.cloudera.org:8080/#/c/15996/2/src/kudu/util/net/net_util.cc File src/kudu/util/net/net_util.cc: http://gerrit.cloudera.org:8080/#/c/15996/2/src/kudu/util/net/net_util.cc@206 PS2, Line 206: RETURN_NOT_OK(GetAddrInfo(host_, hints, op_description, &result)); > Is this something we can put behind a feature flag and leave defaulted to t I'm afraid not. For example, if the user opts into ipv6 but the real address(host_) is ipv4, then the operation will fail. -- To view, visit http://gerrit.cloudera.org:8080/15996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie11e8e03152513284b7088dfba6c905a2fcdf46d Gerrit-Change-Number: 15996 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: helifu Gerrit-Comment-Date: Mon, 08 Jun 2020 06:16:34 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1457 p2: webserver supports for IPv6
helifu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16048 Change subject: KUDU-1457 p2: webserver supports for IPv6 .. KUDU-1457 p2: webserver supports for IPv6 The squeasel's PR is here: https://github.com/cloudera/squeasel/pull/19 Change-Id: Ia6c01839c02c79c08fa40f8e977200bfbbca3504 --- M src/kudu/server/webserver.cc M thirdparty/build-definitions.sh 2 files changed, 10 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/16048/1 -- To view, visit http://gerrit.cloudera.org:8080/16048 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia6c01839c02c79c08fa40f8e977200bfbbca3504 Gerrit-Change-Number: 16048 Gerrit-PatchSet: 1 Gerrit-Owner: helifu
[kudu-CR] KUDU-1457 p1: krpc supports for IPv6
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15996 to look at the new patch set (#3). Change subject: KUDU-1457 p1: krpc supports for IPv6 .. KUDU-1457 p1: krpc supports for IPv6 This patch is intended to support IPv6 by expanding Sockaddr, but please kindly note that: 1) webserver is not included yet; 2) IPv6 link-local is not supported; Change-Id: Ie11e8e03152513284b7088dfba6c905a2fcdf46d --- M src/kudu/rpc/reactor.cc M src/kudu/rpc/rpc-test.cc M src/kudu/util/net/dns_resolver-test.cc M src/kudu/util/net/net_util-test.cc M src/kudu/util/net/net_util.cc M src/kudu/util/net/net_util.h M src/kudu/util/net/sockaddr.cc M src/kudu/util/net/sockaddr.h M src/kudu/util/net/socket-test.cc M src/kudu/util/test_util.cc 10 files changed, 334 insertions(+), 115 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/15996/3 -- To view, visit http://gerrit.cloudera.org:8080/15996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie11e8e03152513284b7088dfba6c905a2fcdf46d Gerrit-Change-Number: 15996 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: helifu
[kudu-CR] KUDU-1457 p1: krpc supports for IPv6
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/15996 ) Change subject: KUDU-1457 p1: krpc supports for IPv6 .. Patch Set 2: I did some tests on a VM box with 1 master + 3 tservers. [root@ecs-1458 release]# ./bin/kudu cluster ksck [::]:7051 Master Summary UUID | Address | Status --+---+- b034de573adb437d92491766ffc04e02 | [::]:7051 | HEALTHY Unusual flags for Master: Flag | Value | Tags | Master -+---+-+- never_fsync | true | unsafe,advanced | all 1 server(s) checked Flags of checked categories for Master: Flag |Value | Master -+-+- builtin_ntp_servers | 0.pool.ntp.org,1.pool.ntp.org,2.pool.ntp.org,3.pool.ntp.org | all 1 server(s) checked time_source | system_unsync | all 1 server(s) checked Tablet Server Summary UUID |Address | Status | Location | Tablet Leaders | Active Scanners --++-+--++- 02fac19fd7a543d5afb640a62ccb5a9e | ecs-1458:31202 | HEALTHY || 2| 0 437a5ec6d9314fc2b95b83e4e7b282da | ecs-1458:31201 | HEALTHY || 4| 0 6da856c3230648cca17687d24ce5aff4 | ecs-1458:31203 | HEALTHY || 2| 0 Tablet Server Location Summary Location | Count --+- | 3 Unusual flags for Tablet Server: Flag | Value | Tags | Tablet Server -+---+-+- never_fsync | true | unsafe,advanced | all 3 server(s) checked Flags of checked categories for Tablet Server: Flag |Value | Tablet Server -+-+- builtin_ntp_servers | 0.pool.ntp.org,1.pool.ntp.org,2.pool.ntp.org,3.pool.ntp.org | all 3 server(s) checked time_source | system_unsync | all 3 server(s) checked Version Summary Version | Servers -+- 1.13.0-SNAPSHOT | all 4 server(s) checked Tablet Summary Summary by table Name| RF | Status | Total Tablets | Healthy | Recovering | Under-replicated | Unavailable ++-+---+-++--+- impala::helf.part | 3 | HEALTHY | 4 | 4 | 0 | 0 | 0 impala::helf.part1 | 3 | HEALTHY | 4 | 4 | 0 | 0 | 0 Tablet Replica Count Summary Statistic| Replica Count +--- Minimum| 8 First Quartile | 8 Median | 8 Third Quartile | 8 Maximum| 8 Total Count Summary | Total Count +- Masters| 1 Tablet Servers | 3 Tables | 2 Tablets| 8 Replicas | 24 == Warnings: == Some masters have unsafe, experimental, or hidden flags set Some tablet servers have unsafe, experimental, or hidden flags set OK [root@ecs-1458 release]# [root@ecs-1458 release]# ./bin/kudu perf loadgen [::]:7051 Using auto-created table 'default.loadgen_auto_218058633ff44beb9554b80621ae15fb' INSERT report rows total: 2000 time total: 12.2919 ms time per row: 0.00614597 ms Dropping auto-created table 'default.loadgen_auto_218058633ff44beb9554b80621ae15fb' [root@ecs-1458 release]# [root@ecs-1458 impala]# impala-shell.sh ... [localhost.localdomain:21000] helf> create table part1 > ( > P_PARTKEY BIGINT, > P_NAME STRING, > P_MFGR STRING, > P_BRAND STRING, > P_TYPE STRING, > P_SIZE INT, > P_CONTAINER STRING, > P_RETAILPRICE DOUBLE, > P_COMMENT STRING, > primary key(P_PARTKEY) > )
[kudu-CR] KUDU-1457 p1: krpc supports for IPv6
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/15996 )
Change subject: KUDU-1457 p1: krpc supports for IPv6
..
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/15996/2/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:
http://gerrit.cloudera.org:8080/#/c/15996/2/src/kudu/rpc/rpc-test.cc@136
PS2, Line 136: testing::Values(false,
true),
> instead of a third bool, I think it makes more sense to change the bool to
Done
http://gerrit.cloudera.org:8080/#/c/15996/2/src/kudu/util/net/dns_resolver-test.cc
File src/kudu/util/net/dns_resolver-test.cc:
http://gerrit.cloudera.org:8080/#/c/15996/2/src/kudu/util/net/dns_resolver-test.cc@64
PS2, Line 64: ASSERT_TRUE(false)
> nit: FAIL()
Done
http://gerrit.cloudera.org:8080/#/c/15996/2/src/kudu/util/net/net_util.h
File src/kudu/util/net/net_util.h:
http://gerrit.cloudera.org:8080/#/c/15996/2/src/kudu/util/net/net_util.h@142
PS2, Line 142: Network(sa_family_t family, uint128_t addr, uint128_t netmask);
> does this make sense for ipv6? I dont know a lot about it but from googling
You mean "uint64" instead? If so, maybe we will lose some info which prevents
address translation while calling "inet_ntop()".
1.IPv6: 8 * 16 = 128 bit.
struct sockaddr_in6 {
sa_family_t sin6_family; /* AF_INET6 */
in_port_t sin6_port; /* port number */
uint32_tsin6_flowinfo; /* IPv6 flow information */
struct in6_addr sin6_addr; /* IPv6 address */
uint32_tsin6_scope_id; /* Scope ID (new in 2.4) */
};
struct in6_addr {
unsigned char s6_addr[16]; /* IPv6 address */
};
2."/64" means prefix length.
http://gerrit.cloudera.org:8080/#/c/15996/2/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/15996/2/src/kudu/util/net/net_util.cc@206
PS2, Line 206: hints.ai_family = AF_UNSPEC;
> will this be an incompatible change of sorts? do we have to worry that clie
yep, it's an incompatible change. But I'm afraid I can't find a better way than
to specify a clear IPv4/IPv6 address(not hostname) for client. What do you
think?
http://gerrit.cloudera.org:8080/#/c/15996/2/src/kudu/util/net/net_util.cc@232
PS2, Line 232: addr->sin6_scope_id =
if_nametoindex(FLAGS_rpc_bind_interface_ipv6.c_str());
> what's this doing?
At first I just wanted to support IPv6 link-local address which requires
scope_id, but now I think we could support it later:)
--
To view, visit http://gerrit.cloudera.org:8080/15996
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie11e8e03152513284b7088dfba6c905a2fcdf46d
Gerrit-Change-Number: 15996
Gerrit-PatchSet: 2
Gerrit-Owner: helifu
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Fri, 05 Jun 2020 07:06:11 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1457 p1: krpc supports for IPv6
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/15996 ) Change subject: KUDU-1457 p1: krpc supports for IPv6 .. Patch Set 2: > Build Started http://jenkins.kudu.apache.org/job/kudu-gerrit/21755/ It seems that the ipv6 is disabled since most to the failures are: Bad status: Network error: connect(2) error: Cannot assign requested address (error 99) But how can we enable the ipv6 while running unit tests? echo 0 > /proc/sys/net/ipv6/conf/all/disable_ipv6 -- To view, visit http://gerrit.cloudera.org:8080/15996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie11e8e03152513284b7088dfba6c905a2fcdf46d Gerrit-Change-Number: 15996 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: helifu Gerrit-Comment-Date: Fri, 29 May 2020 07:34:37 + Gerrit-HasComments: No
[kudu-CR] KUDU-1457 p1: krpc supports for IPv6
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15996 to look at the new patch set (#2). Change subject: KUDU-1457 p1: krpc supports for IPv6 .. KUDU-1457 p1: krpc supports for IPv6 This patch is intended to support IPv6 by expanding Sockaddr, but please kindly note that webserver is not included yet. Change-Id: Ie11e8e03152513284b7088dfba6c905a2fcdf46d --- M src/kudu/rpc/reactor.cc M src/kudu/rpc/rpc-test.cc M src/kudu/util/net/dns_resolver-test.cc M src/kudu/util/net/net_util-test.cc M src/kudu/util/net/net_util.cc M src/kudu/util/net/net_util.h M src/kudu/util/net/sockaddr.cc M src/kudu/util/net/sockaddr.h M src/kudu/util/net/socket-test.cc M src/kudu/util/test_util.cc 10 files changed, 305 insertions(+), 106 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/15996/2 -- To view, visit http://gerrit.cloudera.org:8080/15996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie11e8e03152513284b7088dfba6c905a2fcdf46d Gerrit-Change-Number: 15996 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1457: support for IPv6
helifu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15996 Change subject: KUDU-1457: support for IPv6 .. KUDU-1457: support for IPv6 This patch is intended to support IPv6 by expanding Sockaddr, but please kindly note that webserver is not included. Change-Id: Ie11e8e03152513284b7088dfba6c905a2fcdf46d --- M src/kudu/rpc/reactor.cc M src/kudu/rpc/rpc-test.cc M src/kudu/util/net/net_util-test.cc M src/kudu/util/net/net_util.cc M src/kudu/util/net/net_util.h M src/kudu/util/net/sockaddr.cc M src/kudu/util/net/sockaddr.h M src/kudu/util/net/socket-test.cc M src/kudu/util/test_util.cc 9 files changed, 294 insertions(+), 104 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/15996/1 -- To view, visit http://gerrit.cloudera.org:8080/15996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie11e8e03152513284b7088dfba6c905a2fcdf46d Gerrit-Change-Number: 15996 Gerrit-PatchSet: 1 Gerrit-Owner: helifu
[kudu-CR] KUDU-3007 (2/3): Add atomicops-internals-arm64.h into tree to support atomicops on aarch64
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/15875 ) Change subject: KUDU-3007 (2/3): Add atomicops-internals-arm64.h into tree to support atomicops on aarch64 .. Patch Set 2: Code-Review+1 > LGTM :) -- To view, visit http://gerrit.cloudera.org:8080/15875 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ida2e9cbe3018fa34c5218cc191fd8cfece869e0b Gerrit-Change-Number: 15875 Gerrit-PatchSet: 2 Gerrit-Owner: liusheng Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Fri, 08 May 2020 09:08:23 + Gerrit-HasComments: No
[kudu-CR] KUDU-3007 (2/3): Add atomicops-internals-arm64.h into tree to support atomicops on aarch64
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/15875 ) Change subject: KUDU-3007 (2/3): Add atomicops-internals-arm64.h into tree to support atomicops on aarch64 .. Patch Set 2: LGTM :) -- To view, visit http://gerrit.cloudera.org:8080/15875 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ida2e9cbe3018fa34c5218cc191fd8cfece869e0b Gerrit-Change-Number: 15875 Gerrit-PatchSet: 2 Gerrit-Owner: liusheng Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Fri, 08 May 2020 09:08:11 + Gerrit-HasComments: No
[kudu-CR] KUDU-3007 (1/3): Import sse2neon.h into tree to Adapt SSE intrinsics on aarch64
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/15874 ) Change subject: KUDU-3007 (1/3): Import sse2neon.h into tree to Adapt SSE intrinsics on aarch64 .. Patch Set 2: Code-Review+1 LGTM :) -- To view, visit http://gerrit.cloudera.org:8080/15874 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7af0b1ce17d6a3a3d87de793ba3973c393dc2088 Gerrit-Change-Number: 15874 Gerrit-PatchSet: 2 Gerrit-Owner: liusheng Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Fri, 08 May 2020 09:06:53 + Gerrit-HasComments: No
[kudu-CR] KUDU-3007 (2/3): Add atomicops-internals-arm64.h into tree to support atomicops on aarch64
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/15875 ) Change subject: KUDU-3007 (2/3): Add atomicops-internals-arm64.h into tree to support atomicops on aarch64 .. Patch Set 1: I find an example of adding arm64 atomicOps: https://github.com/protocolbuffers/protobuf/commit/2ca19bd8066821a56f193e7fca47139b25c617ad -- To view, visit http://gerrit.cloudera.org:8080/15875 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ida2e9cbe3018fa34c5218cc191fd8cfece869e0b Gerrit-Change-Number: 15875 Gerrit-PatchSet: 1 Gerrit-Owner: liusheng Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Fri, 08 May 2020 07:40:29 + Gerrit-HasComments: No
[kudu-CR] KUDU-3007 (3/3): Support building and running Kudu on aarch64 platform
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14964 ) Change subject: KUDU-3007 (3/3): Support building and running Kudu on aarch64 platform .. Patch Set 19: (3 comments) http://gerrit.cloudera.org:8080/#/c/14964/19/src/kudu/gutil/atomicops.h File src/kudu/gutil/atomicops.h: http://gerrit.cloudera.org:8080/#/c/14964/19/src/kudu/gutil/atomicops.h@87 PS19, Line 87: #elif defined(__GNUC__) && defined(__aarch64__) : #include "kudu/gutil/atomicops-internals-arm64.h" // IWYU pragma: export Can you move these two lines of code to patch2: https://gerrit.cloudera.org/#/c/15875/ ? http://gerrit.cloudera.org:8080/#/c/14964/19/src/kudu/util/char_util.cc File src/kudu/util/char_util.cc: http://gerrit.cloudera.org:8080/#/c/14964/19/src/kudu/util/char_util.cc@20 PS19, Line 20: #ifdef __aarch64__ : #include "kudu/util/sse2neon.h" : #else : #include : #include : #endif //__aarch64__ Can you move these to patch1: https://gerrit.cloudera.org/#/c/15874/ ? http://gerrit.cloudera.org:8080/#/c/14964/19/src/kudu/util/debug-util.cc File src/kudu/util/debug-util.cc: http://gerrit.cloudera.org:8080/#/c/14964/19/src/kudu/util/debug-util.cc@46 PS19, Line 46: #ifdef __aarch64__ : #include : #else : #include : #endif //__aarch64__ The "libunwind.h" has included "libunwind-aarch64.h": https://github.com/libunwind/libunwind/blob/v1.3-stable/include/libunwind.h.in It means the modifications here are not necessary. -- To view, visit http://gerrit.cloudera.org:8080/14964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2953519c5d28de17e6b2bb7094abab0c1cd12c97 Gerrit-Change-Number: 14964 Gerrit-PatchSet: 19 Gerrit-Owner: liusheng Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: RuiChen Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: helifu Gerrit-Reviewer: liusheng Gerrit-Comment-Date: Fri, 08 May 2020 07:37:42 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3007. Support building Kudu on aarch64 platform
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14964 ) Change subject: KUDU-3007. Support building Kudu on aarch64 platform .. Patch Set 18: > Could you separate all third-party files imported as-is into a > separate change list and then rebase this patch on top of that? > > Having its own changelist for those third-party files will help to > separate the changes in the Kudu's and third-party code to support > Kudu building and running on aarch64. Agree with Alexey's comment. As we discussed before, small,independent and incremental patches will be more conducive to review. //By the way, good job @liusheng :) -- To view, visit http://gerrit.cloudera.org:8080/14964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2953519c5d28de17e6b2bb7094abab0c1cd12c97 Gerrit-Change-Number: 14964 Gerrit-PatchSet: 18 Gerrit-Owner: liusheng Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: RuiChen Gerrit-Reviewer: helifu Gerrit-Reviewer: liusheng Gerrit-Comment-Date: Wed, 06 May 2020 12:05:30 + Gerrit-HasComments: No
[kudu-CR] KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/15034 ) Change subject: KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side .. Patch Set 13: Code-Review+2 > Leaving open in case Lifu (or others) have more comments. Thanks. It looks good to me apart from the impact of the evaluation (it's very CPU intensive). It looks good to me -- To view, visit http://gerrit.cloudera.org:8080/15034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a Gerrit-Change-Number: 15034 Gerrit-PatchSet: 13 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao Gerrit-Reviewer: helifu Gerrit-Comment-Date: Fri, 14 Feb 2020 03:11:51 + Gerrit-HasComments: No
[kudu-CR] [client] KUDU-2483 Add IN Bloom filter predicate to C++ client
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/15122 ) Change subject: [client] KUDU-2483 Add IN Bloom filter predicate to C++ client .. Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/15122/6/src/kudu/client/scan_predicate.h File src/kudu/client/scan_predicate.h: http://gerrit.cloudera.org:8080/#/c/15122/6/src/kudu/client/scan_predicate.h@95 PS6, Line 95: Slice It's a good definition here. Once I used "const KuduValue* value", then a lot of useless code was introduced into my implementation. From the user's perspective, some comments for different column type, i.e. bool/int16/int32 ... will be welcome. http://gerrit.cloudera.org:8080/#/c/15122/6/src/kudu/client/scan_predicate.h@130 PS6, Line 130: int I think "size_t" will be better than "int". http://gerrit.cloudera.org:8080/#/c/15122/6/src/kudu/client/scan_predicate.cc File src/kudu/client/scan_predicate.cc: http://gerrit.cloudera.org:8080/#/c/15122/6/src/kudu/client/scan_predicate.cc@260 PS6, Line 260: Build(KuduBloomFilter** bloom_filter_out) Maybe return Status::InvalidArgument as soon as possible if the 'num_keys' is less or equal than 0. -- To view, visit http://gerrit.cloudera.org:8080/15122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4aa235a4c933ebce0bf3ec7fcb135098eccc4ea4 Gerrit-Change-Number: 15122 Gerrit-PatchSet: 6 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao Gerrit-Reviewer: helifu Gerrit-Comment-Date: Tue, 11 Feb 2020 12:58:43 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/15034 ) Change subject: KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side .. Patch Set 13: > > In general, the bloom filter is used as run-time filter, > especially in a hash-join case. For a computer engine, such as > impala, it will wait 1 second(default) to produce the bloom filter > and then push it down. So, I have another suggestion: maybe we can > push it down in the middle of the scan while the filter is not > arrived within the specified interval. It should be in the next > patch^_^ > > > > computer engine -> kudu client -> kudu server > >[1][2][3] > >throw away > >[0] > > That's interesting: are you suggesting we need the ability to add a > bloom filter predicate to an ongoing scan, rather than just when > starting a new scan? There's a lot of existing machinery that > treats predicates as just another aspect of scan configuration > (along with e.g. projections), to be applied to a new scanner but > immutable after that. I think it'd be a fair amount of work to > change that. > > As an alternative, would it be possible to delay the onset of a > Kudu scan from Impala until the bloom filter can be constructed? Or > will doing that stall the entire query pipeline? Nope, I mean that adding a bloom filter predicate to an ongoing scan is a new independent feature, and we can implement it in the next patch. No, it's impossible to delay the onset of a kudu scan. For example, if the time to wait for the bloom filter is greater than the time to scan all of the data, the waiting would not be the best option. -- To view, visit http://gerrit.cloudera.org:8080/15034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a Gerrit-Change-Number: 15034 Gerrit-PatchSet: 13 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao Gerrit-Reviewer: helifu Gerrit-Comment-Date: Mon, 10 Feb 2020 01:49:09 + Gerrit-HasComments: No
[kudu-CR] KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/15034 ) Change subject: KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side .. Patch Set 12: (3 comments) Good job, Bankim! In general, the bloom filter is used as run-time filter, especially in a hash-join case. For a computer engine, such as impala, it will wait 1 second(default) to produce the bloom filter and then push it down. So, I have another suggestion: maybe we can push it down in the middle of the scan while the filter is not arrived within the specified interval. It should be in the next patch^_^ computer engine -> kudu client -> kudu server [1][2][3] throw away [0] http://gerrit.cloudera.org:8080/#/c/15034/12/src/kudu/common/column_predicate.h File src/kudu/common/column_predicate.h: http://gerrit.cloudera.org:8080/#/c/15034/12/src/kudu/common/column_predicate.h@379 PS12, Line 379: std::vector bloom_filters_; In my understanding, there should be at most one bloom filter in a predicate. Even if in a particular case, someone wants to push down some bloom filters on the same column at the same time, these bloom filters should have the same "ndv" and "log_space_bytes" for the current tablet/table, and all of them could be merged into one(AND operation, OR operation is not supported yet.). But in fact, nobody will do that since the bloom filter will take lots of CPU cycles while evaluating. So, could you please share your thoughts? http://gerrit.cloudera.org:8080/#/c/15034/11/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: http://gerrit.cloudera.org:8080/#/c/15034/11/src/kudu/common/column_predicate.cc@113 PS11, Line 113: std:: remove "std::" http://gerrit.cloudera.org:8080/#/c/15034/12/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: http://gerrit.cloudera.org:8080/#/c/15034/12/src/kudu/common/column_predicate.cc@119 PS12, Line 119: pred.Simplify(); I think we need to check the size(GetSpaceUsed) of bloom filter because the default value of 'rpc_max_message_size' is 50Mib. -- To view, visit http://gerrit.cloudera.org:8080/15034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a Gerrit-Change-Number: 15034 Gerrit-PatchSet: 12 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: ZhangYao Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 06 Feb 2020 12:31:40 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2986 p3: hide the values while there is old version
Hello Alexey Serbin, Kudu Jenkins, Volodymyr Verovkin, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15071 to look at the new patch set (#3). Change subject: KUDU-2986 p3: hide the values while there is old version .. KUDU-2986 p3: hide the values while there is old version The old version will not report statistics in the heartbeat msg, so the aggregated values are not accurate and we need to hide them. Change-Id: I691bb8c447c878471ed795028f0aab89a39bd946 --- M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/table_statistics-internal.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master-test.cc M src/kudu/master/master_path_handlers.cc M src/kudu/master/table_metrics.cc M src/kudu/master/table_metrics.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/ts_tablet_manager.cc 13 files changed, 233 insertions(+), 95 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/15071/3 -- To view, visit http://gerrit.cloudera.org:8080/15071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I691bb8c447c878471ed795028f0aab89a39bd946 Gerrit-Change-Number: 15071 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2986 p3: hide the values while there is old version
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/15071 )
Change subject: KUDU-2986 p3: hide the values while there is old version
..
Patch Set 2:
(17 comments)
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/client/table_statistics-internal.h
File src/kudu/client/table_statistics-internal.h:
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/client/table_statistics-internal.h@37
PS2, Line 37: : on_disk_size_(std::move(on_disk_size))
: , live_row_count_(std::move(live_row_count)) {
> nit: aside from the essence of this change, I think the way how it was form
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/catalog_manager.h@336
PS2, Line 336: InvalidMetrics
> Rename into 'InvalidateMetrics' ?
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/catalog_manager.cc@4285
PS2, Line 4285: its own
> it owns ?
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/catalog_manager.cc@4295
PS2, Line 4295: hide
> hidden
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/catalog_manager.cc@5672
PS2, Line 5672: uint64_t on_disk_size = new_stats.on_disk_size();
> Does it make sense to add
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/catalog_manager.cc@5685
PS2, Line 5685: // Do not update the metric value when it is invisible,
because the update
: // operation will trigger the metric to be visible.
> I think you can remove this or move this to the generic comment on the 'on
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/catalog_manager.cc@5703
PS2, Line 5703: new
> newly
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/catalog_manager.cc@5705
PS2, Line 5705: if (metrics_->ContainsTabletNoLiveRowCount(tablet_id)) {
: if (new_stats.has_live_row_count()) {
> nit: combine into one clause?
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/catalog_manager.cc@5721
PS2, Line 5721: } else {
: // It is case 1 and the new_stats without
'live_row_count' will keep coming.
: }
: } else {
: // Do not update the metric value when it is invisible.
: }
> I think you can remove this, maybe moving the comments to the generic comme
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/master-test.cc@1983
PS2, Line 1983: tables[0]
> nit: for the ease of referencing this single table, maybe declare a referen
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/master-test.cc@1986
PS2, Line 1986: tablets.back()
> Add an assert to make sure 'tablets' is not empty.
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/master_path_handlers.cc
File src/kudu/master/master_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/master_path_handlers.cc@502
PS2, Line 502: we need to show them explicitly
> This reads a bit confusing. Should it be "show their values as 'N/A'"?
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/table_metrics.h
File src/kudu/master/table_metrics.h:
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/table_metrics.h@20
PS2, Line 20: #include
> nit: use
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/table_metrics.h@66
PS2, Line 66: IDS
> nit: Identifiers ?
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/table_metrics.h@68
PS2, Line 68: IDs
> nit: Identifiers ?
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/table_metrics.cc
File src/kudu/master/table_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/master/table_metrics.cc@52
PS2, Line 52: ,
> nit: correct the spacing
Done
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/15071/2/src/kudu/tserver/ts_tablet_manager.cc@1629
PS2, Line 1629: !dirty_tablets.empty()
> Good catch! Even if the MarkTabletDirty() doesn't do much if the container
:)
--
To view, visit http://gerrit.cloudera.org:8080/15071
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I691bb8c447c878471ed795028f0aab89a39bd946
Gerrit-Change-Number: 15071
Gerrit-PatchSet: 2
Gerrit-Owner: helifu
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit
[kudu-CR] KUDU-2986 p3: hide the values while there is old version
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15071 to look at the new patch set (#2). Change subject: KUDU-2986 p3: hide the values while there is old version .. KUDU-2986 p3: hide the values while there is old version The old version will not report statistics in the heartbeat msg, so the aggregated values are not accurate and we need to hide them. Change-Id: I691bb8c447c878471ed795028f0aab89a39bd946 --- M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/table_statistics-internal.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master-test.cc M src/kudu/master/master_path_handlers.cc M src/kudu/master/table_metrics.cc M src/kudu/master/table_metrics.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/ts_tablet_manager.cc 13 files changed, 240 insertions(+), 96 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/15071/2 -- To view, visit http://gerrit.cloudera.org:8080/15071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I691bb8c447c878471ed795028f0aab89a39bd946 Gerrit-Change-Number: 15071 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2986 p3: hide the values while there is old version
helifu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15071 Change subject: KUDU-2986 p3: hide the values while there is old version .. KUDU-2986 p3: hide the values while there is old version The old version will not report statistics in the heartbeat msg, so the aggregated values are not accurate and we need to hide them. Change-Id: I691bb8c447c878471ed795028f0aab89a39bd946 --- M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/table_statistics-internal.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master-test.cc M src/kudu/master/master_path_handlers.cc M src/kudu/master/table_metrics.cc M src/kudu/master/table_metrics.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/tserver/ts_tablet_manager.cc 12 files changed, 230 insertions(+), 91 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/15071/1 -- To view, visit http://gerrit.cloudera.org:8080/15071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I691bb8c447c878471ed795028f0aab89a39bd946 Gerrit-Change-Number: 15071 Gerrit-PatchSet: 1 Gerrit-Owner: helifu
[kudu-CR] [util] Import Impala's block based BloomFilter
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14745 ) Change subject: [util] Import Impala's block based BloomFilter .. Patch Set 9: (4 comments) http://gerrit.cloudera.org:8080/#/c/14745/9/src/kudu/util/block_bloom_filter.cc File src/kudu/util/block_bloom_filter.cc: http://gerrit.cloudera.org:8080/#/c/14745/9/src/kudu/util/block_bloom_filter.cc@83 PS9, Line 83: directory_mask_ = (1ULL << std::min(63, log_num_buckets_)) - 1; seems 'log_num_buckets_' is always less than 63 according to Line77. http://gerrit.cloudera.org:8080/#/c/14745/9/src/kudu/util/block_bloom_filter.cc@108 PS9, Line 108: new_bucket[i] = : (kRehash[i] * hash) >> ((1 << kLogBucketWordBits) - kLogBucketWordBits); The code would look nicer if we didn't wrap(newline) it, what do you think? http://gerrit.cloudera.org:8080/#/c/14745/9/src/kudu/util/block_bloom_filter.cc@113 PS9, Line 113: __m128i new_bucket_sse = : _mm_load_si128(reinterpret_cast<__m128i*>(new_bucket + 4 * i)); the same. http://gerrit.cloudera.org:8080/#/c/14745/9/src/kudu/util/block_bloom_filter.cc@124 PS9, Line 124: BucketWord hval = : (kRehash[i] * hash) >> ((1 << kLogBucketWordBits) - kLogBucketWordBits); the same. -- To view, visit http://gerrit.cloudera.org:8080/14745 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89c54a051c5093cf5fb81481a47a0a6677d7d906 Gerrit-Change-Number: 14745 Gerrit-PatchSet: 9 Gerrit-Owner: Bankim Bhavsar Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: helifu Gerrit-Comment-Date: Tue, 10 Dec 2019 09:58:08 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14601
to look at the new patch set (#10).
Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's
Web UI
..
KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI
Hide the 'live_row_count' metric in the master's Web UI while
there is any tablet which doesn't support live row count. And
unhide it after the legacy tablets are all deleted.
http://master:8051/metrics?ids=5448c0a8658f467ea373d9044d567011
[
{
"type": "table",
"id": "5448c0a8658f467ea373d9044d567011",
"attributes": {
"table_name": "testtable"
},
"metrics": [
{
"name": "on_disk_size",
"value": 8417243
}
]
}
]
Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/util/metrics.h
3 files changed, 106 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/14601/10
--
To view, visit http://gerrit.cloudera.org:8080/14601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
Gerrit-Change-Number: 14601
Gerrit-PatchSet: 10
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14601 ) Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI .. Patch Set 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/14601/9/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/14601/9/src/kudu/master/catalog_manager.cc@5599 PS9, Line 5599: Record the tablet > What does 'record the tablet' mean? Could you clarify? Done http://gerrit.cloudera.org:8080/#/c/14601/9/src/kudu/master/catalog_manager.cc@5599 PS9, Line 5599: // Record the tablet while the new_stats doesn't support 'live_row_count'. But the : // new_stats will keep coming, so it's necessary to limit the recording operation : // to fire only once. : // Here we use the 'on_disk_size' metric of the old_stats when it is transitioned : // from "uninitialized stats" to "has_on_disk_size()". > I think it makes sense to move this inside the 'else if ()' clause below: t Done http://gerrit.cloudera.org:8080/#/c/14601/9/src/kudu/util/metrics.h File src/kudu/util/metrics.h: http://gerrit.cloudera.org:8080/#/c/14601/9/src/kudu/util/metrics.h@786 PS9, Line 786: // Return true if this metric is invisible otherwise false. > nit: add an empty line between the end of the previous function definition Done -- To view, visit http://gerrit.cloudera.org:8080/14601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533 Gerrit-Change-Number: 14601 Gerrit-PatchSet: 9 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 04 Dec 2019 02:39:02 + Gerrit-HasComments: Yes
[kudu-CR] tools: support for changing column comment
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14814 to look at the new patch set (#3). Change subject: tools: support for changing column comment .. tools: support for changing column comment Change-Id: I8bfc7f5b1c4f89d01d2f5145b648d15d90f95dfb --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 2 files changed, 58 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/14814/3 -- To view, visit http://gerrit.cloudera.org:8080/14814 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8bfc7f5b1c4f89d01d2f5145b648d15d90f95dfb Gerrit-Change-Number: 14814 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] tools: support for changing column comment
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/14814 )
Change subject: tools: support for changing column comment
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14814/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/14814/2/src/kudu/tools/kudu-tool-test.cc@1113
PS2, Line 1113: const vector kTableModeRegexes = {
> Update this.
Done
--
To view, visit http://gerrit.cloudera.org:8080/14814
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8bfc7f5b1c4f89d01d2f5145b648d15d90f95dfb
Gerrit-Change-Number: 14814
Gerrit-PatchSet: 2
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Mon, 02 Dec 2019 06:33:36 +
Gerrit-HasComments: Yes
[kudu-CR] tools: support for changing column comment
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14814 to look at the new patch set (#2). Change subject: tools: support for changing column comment .. tools: support for changing column comment Change-Id: I8bfc7f5b1c4f89d01d2f5145b648d15d90f95dfb --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 2 files changed, 40 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/14814/2 -- To view, visit http://gerrit.cloudera.org:8080/14814 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8bfc7f5b1c4f89d01d2f5145b648d15d90f95dfb Gerrit-Change-Number: 14814 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] tool: support for changing column comment
helifu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14814 Change subject: tool: support for changing column comment .. tool: support for changing column comment Change-Id: I8bfc7f5b1c4f89d01d2f5145b648d15d90f95dfb --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_table.cc 2 files changed, 40 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/14814/1 -- To view, visit http://gerrit.cloudera.org:8080/14814 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8bfc7f5b1c4f89d01d2f5145b648d15d90f95dfb Gerrit-Change-Number: 14814 Gerrit-PatchSet: 1 Gerrit-Owner: helifu
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14601
to look at the new patch set (#9).
Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's
Web UI
..
KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI
Hide the 'live_row_count' metric in the master's Web UI while
there is any tablet which doesn't support live row count. And
unhide it after the legacy tablets are all deleted.
http://master:8051/metrics?ids=5448c0a8658f467ea373d9044d567011
[
{
"type": "table",
"id": "5448c0a8658f467ea373d9044d567011",
"attributes": {
"table_name": "testtable"
},
"metrics": [
{
"name": "on_disk_size",
"value": 8417243
}
]
}
]
Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/util/metrics.h
3 files changed, 105 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/14601/9
--
To view, visit http://gerrit.cloudera.org:8080/14601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
Gerrit-Change-Number: 14601
Gerrit-PatchSet: 9
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/14601 )
Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's
Web UI
..
Patch Set 8:
> Build Failed
>
> http://jenkins.kudu.apache.org/job/kudu-gerrit/19705/ : FAILURE
The failed java test "org.apache.kudu.client.TestKuduScanner" was caused by a
timeout, almost 2 seconds, and it should be a unrelated test case.
02:17:51.809 [INFO - cluster stderr printer] (MiniKuduCluster.java:582) I1129
02:17:51.809465 3397 catalog_manager.cc:2380] Servicing AlterTable request
from {username='slave'} at 127.0.0.1:40340:
... ...
02:17:53.798 [INFO - cluster stderr printer] (MiniKuduCluster.java:582) I1129
02:17:53.798640 3379 catalog_manager.cc:3512] TS
1edaabdc4c2d4ff2b83710e5daa542be (127.3.72.1:44621): tablet
6f6d653755164d6aa58684952ea6e28e (table testOpenScanWithDroppedPartition
[id=1c2df3989f554d2eb51b93dbef3d5bd1]) successfully deleted
--
To view, visit http://gerrit.cloudera.org:8080/14601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
Gerrit-Change-Number: 14601
Gerrit-PatchSet: 8
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Fri, 29 Nov 2019 02:54:20 +
Gerrit-HasComments: No
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14601
to look at the new patch set (#8).
Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's
Web UI
..
KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI
Hide the 'live_row_count' metric in the master's Web UI while
there is any tablet which doesn't support live row count. And
unhide it after the legacy tablets are all deleted.
http://master:8051/metrics?ids=5448c0a8658f467ea373d9044d567011
[
{
"type": "table",
"id": "5448c0a8658f467ea373d9044d567011",
"attributes": {
"table_name": "testtable"
},
"metrics": [
{
"name": "on_disk_size",
"value": 8417243
}
]
}
]
Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/util/metrics.h
3 files changed, 105 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/14601/8
--
To view, visit http://gerrit.cloudera.org:8080/14601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
Gerrit-Change-Number: 14601
Gerrit-PatchSet: 8
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14601 ) Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI .. Patch Set 7: > Build Failed > > http://jenkins.kudu.apache.org/job/kudu-gerrit/19702/ : FAILURE It doesn't look like the unit test itself failed. Let me trigger it again. -- To view, visit http://gerrit.cloudera.org:8080/14601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533 Gerrit-Change-Number: 14601 Gerrit-PatchSet: 7 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: helifu Gerrit-Comment-Date: Fri, 29 Nov 2019 01:55:35 + Gerrit-HasComments: No
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14601
to look at the new patch set (#7).
Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's
Web UI
..
KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI
Hide the 'live_row_count' metric in the master's Web UI while
there is any tablet which doesn't support live row count. And
unhide it after the legacy tablets are all deleted.
http://master:8051/metrics?ids=5448c0a8658f467ea373d9044d567011
[
{
"type": "table",
"id": "5448c0a8658f467ea373d9044d567011",
"attributes": {
"table_name": "testtable"
},
"metrics": [
{
"name": "on_disk_size",
"value": 8417243
}
]
}
]
Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/util/metrics.h
3 files changed, 105 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/14601/7
--
To view, visit http://gerrit.cloudera.org:8080/14601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
Gerrit-Change-Number: 14601
Gerrit-PatchSet: 7
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14601 ) Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI .. Patch Set 6: (3 comments) Thanks :) http://gerrit.cloudera.org:8080/#/c/14601/6/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/14601/6/src/kudu/master/catalog_manager.cc@5591 PS6, Line 5591: will trigger > nit: makes Done http://gerrit.cloudera.org:8080/#/c/14601/6/src/kudu/master/catalog_manager.cc@5599 PS6, Line 5599: !old_stats.has_on_disk_size() > It's not easy to follow the logic of these conditionals without knowing the Done http://gerrit.cloudera.org:8080/#/c/14601/6/src/kudu/master/master-test.cc File src/kudu/master/master-test.cc: http://gerrit.cloudera.org:8080/#/c/14601/6/src/kudu/master/master-test.cc@1824 PS6, Line 1824: //old_stats.set_on_disk_size(1); > Remove this commented out line, it's not needed. Done -- To view, visit http://gerrit.cloudera.org:8080/14601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533 Gerrit-Change-Number: 14601 Gerrit-PatchSet: 6 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 28 Nov 2019 15:06:25 + Gerrit-HasComments: Yes
[kudu-CR] tserver: expose the number of round trips for a scanner
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14803 ) Change subject: tserver: expose the number of round trips for a scanner .. Patch Set 3: > Sorry, forgot to mention in my first review, but could you include > a screenshot in the gerrit change (or, if permalinked, in the > commit message) showing the new /scans page in action? Sure. https://slack-files.com/T0CPU21PZ-FR1Q3S3RA-39cfe428b7 -- To view, visit http://gerrit.cloudera.org:8080/14803 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia596049c156be5f79e55ac7082488ba1ba462441 Gerrit-Change-Number: 14803 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 28 Nov 2019 02:14:51 + Gerrit-HasComments: No
[kudu-CR] tserver: expose the number of round trips for a scanner
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14803 to look at the new patch set (#3). Change subject: tserver: expose the number of round trips for a scanner .. tserver: expose the number of round trips for a scanner Change-Id: Ia596049c156be5f79e55ac7082488ba1ba462441 --- M src/kudu/tserver/tserver_path_handlers.cc M www/scans.mustache 2 files changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/14803/3 -- To view, visit http://gerrit.cloudera.org:8080/14803 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia596049c156be5f79e55ac7082488ba1ba462441 Gerrit-Change-Number: 14803 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] tserver: expose the number of round trips for a scanner
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14803 ) Change subject: tserver: expose the number of round trips for a scanner .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/14803/2/src/kudu/tserver/tserver_path_handlers.cc File src/kudu/tserver/tserver_path_handlers.cc: http://gerrit.cloudera.org:8080/#/c/14803/2/src/kudu/tserver/tserver_path_handlers.cc@546 PS2, Line 546: number_of_round_trip > Nit: num_round_trips Done -- To view, visit http://gerrit.cloudera.org:8080/14803 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia596049c156be5f79e55ac7082488ba1ba462441 Gerrit-Change-Number: 14803 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 28 Nov 2019 01:32:10 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14601
to look at the new patch set (#6).
Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's
Web UI
..
KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI
Hide the 'live_row_count' metric in the master's Web UI while
there is any tablet which doesn't support live row count. And
unhide it after the legacy tablets are all deleted.
http://master:8051/metrics?ids=5448c0a8658f467ea373d9044d567011
[
{
"type": "table",
"id": "5448c0a8658f467ea373d9044d567011",
"attributes": {
"table_name": "testtable"
},
"metrics": [
{
"name": "on_disk_size",
"value": 8417243
}
]
}
]
Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/util/metrics.h
3 files changed, 97 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/14601/6
--
To view, visit http://gerrit.cloudera.org:8080/14601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
Gerrit-Change-Number: 14601
Gerrit-PatchSet: 6
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: helifu
[kudu-CR] tserver: expose the number of round trips for a scanner
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14803 to look at the new patch set (#2). Change subject: tserver: expose the number of round trips for a scanner .. tserver: expose the number of round trips for a scanner Change-Id: Ia596049c156be5f79e55ac7082488ba1ba462441 --- M src/kudu/tserver/tserver_path_handlers.cc M www/scans.mustache 2 files changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/14803/2 -- To view, visit http://gerrit.cloudera.org:8080/14803 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia596049c156be5f79e55ac7082488ba1ba462441 Gerrit-Change-Number: 14803 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] tserver: expose the number of round trips for a scanner
helifu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14803 Change subject: tserver: expose the number of round trips for a scanner .. tserver: expose the number of round trips for a scanner Change-Id: Ia596049c156be5f79e55ac7082488ba1ba462441 --- M src/kudu/tserver/tserver_path_handlers.cc M www/scans.mustache 2 files changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/14803/1 -- To view, visit http://gerrit.cloudera.org:8080/14803 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia596049c156be5f79e55ac7082488ba1ba462441 Gerrit-Change-Number: 14803 Gerrit-PatchSet: 1 Gerrit-Owner: helifu
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14601
to look at the new patch set (#5).
Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's
Web UI
..
KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI
Hide the 'live_row_count' metric in the master's Web UI while
there is any tablet which doesn't support live row count. And
unhide it after the legacy tablets are all deleted.
http://master:8051/metrics?ids=5448c0a8658f467ea373d9044d567011
[
{
"type": "table",
"id": "5448c0a8658f467ea373d9044d567011",
"attributes": {
"table_name": "testtable"
},
"metrics": [
{
"name": "on_disk_size",
"value": 8417243
}
]
}
]
Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/util/metrics.h
3 files changed, 97 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/14601/5
--
To view, visit http://gerrit.cloudera.org:8080/14601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
Gerrit-Change-Number: 14601
Gerrit-PatchSet: 5
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2919 p1: trash is supported while dropping tables
Hello Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14566 to look at the new patch set (#4). Change subject: KUDU-2919 p1: trash is supported while dropping tables .. KUDU-2919 p1: trash is supported while dropping tables 1) Add a new field to persist the deletion time for the trashed table/tablets on the master; 2) Add a new boolean variable to represent the trash state for tablets on the tserver; 3) The trash state of the tablet can be synchronized by heartbeat messages (master -> tserver), even if the tserver restarts; 4) The CatalogManagerBgTasks will delete the trash tables that are timeout; 5) Add a new RollbackTable API to roll back the trashed table. Change-Id: I112da81727db6f64026a54311d88e436c34ad6d8 --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/master_proxy_rpc.cc M src/kudu/integration-tests/delete_table-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/master/master_service.h M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tablet_service.h M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/tserver/tserver_admin.proto 18 files changed, 915 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/14566/4 -- To view, visit http://gerrit.cloudera.org:8080/14566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I112da81727db6f64026a54311d88e436c34ad6d8 Gerrit-Change-Number: 14566 Gerrit-PatchSet: 4 Gerrit-Owner: helifu Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-1929 p1: trash is supported while dropping tables
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14566 ) Change subject: KUDU-1929 p1: trash is supported while dropping tables .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/14566/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14566/3//COMMIT_MSG@7 PS3, Line 7: KUDU-1929 p1: trash is supported while dropping tables > it should be KUDU-2919 Thank you for pointing out the mistake, Attila^_^ -- To view, visit http://gerrit.cloudera.org:8080/14566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I112da81727db6f64026a54311d88e436c34ad6d8 Gerrit-Change-Number: 14566 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Tue, 19 Nov 2019 00:46:20 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14601
to look at the new patch set (#4).
Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's
Web UI
..
KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI
Hide the 'live_row_count' metric in the master's Web UI while
there is any tablet which doesn't support live row count. And
unhide it after the legacy tablets are all deleted.
http://master:8051/metrics?ids=5448c0a8658f467ea373d9044d567011
[
{
"type": "table",
"id": "5448c0a8658f467ea373d9044d567011",
"attributes": {
"table_name": "testtable"
},
"metrics": [
{
"name": "on_disk_size",
"value": 8417243
}
]
}
]
Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/util/metrics.h
3 files changed, 97 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/14601/4
--
To view, visit http://gerrit.cloudera.org:8080/14601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
Gerrit-Change-Number: 14601
Gerrit-PatchSet: 4
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14601 ) Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/14601/3/src/kudu/master/table_metrics.cc File src/kudu/master/table_metrics.cc: http://gerrit.cloudera.org:8080/#/c/14601/3/src/kudu/master/table_metrics.cc@36 PS3, Line 36: METRIC_DEFINE_gauge_string(table, live_row_count, "Table Live Row count", > I understand the desire to show "N/A" when a table doesn't support live row So, should I hide this metric just like on the tserver web UI? -- To view, visit http://gerrit.cloudera.org:8080/14601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533 Gerrit-Change-Number: 14601 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: helifu Gerrit-Comment-Date: Mon, 18 Nov 2019 07:34:52 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14601
to look at the new patch set (#3).
Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's
Web UI
..
KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI
The 'live_row_count' metric should be N/A(invalid) in master's Web UI
while there is any tablet which doesn't support live row count.
http://master:8051/metrics?ids=5448c0a8658f467ea373d9044d567011
[
{
"type": "table",
"id": "5448c0a8658f467ea373d9044d567011",
"attributes": {
"table_name": "testtable"
},
"metrics": [
{
"name": "live_row_count",
"value": "N/A"
},
{
"name": "on_disk_size",
"value": 8417243
}
]
}
]
Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
---
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/table_metrics.h
6 files changed, 104 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/14601/3
--
To view, visit http://gerrit.cloudera.org:8080/14601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
Gerrit-Change-Number: 14601
Gerrit-PatchSet: 3
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: helifu
[kudu-CR] [metrics] Add metric severity level
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14630 ) Change subject: [metrics] Add metric severity level .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/14630/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14630/4//COMMIT_MSG@26 PS4, Line 26: Additionally a configuration flag, `metrics_default_level`, was added with : a default value of `debug` to include all metrics by default but allow users : to configure the default to drastically reduce the metrics ouput. Instead of adding a new gflag to the kudu server to determine the metrics to return, how about adding a new filter from the Web URL? I mean we can config the severity level for the metrics, but it is better to let the client decide which level of metrics they need. -- To view, visit http://gerrit.cloudera.org:8080/14630 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5aa7d2323bb75700104c348a3ae859fc449e1715 Gerrit-Change-Number: 14630 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 06 Nov 2019 02:38:31 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/14601 )
Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's
Web UI
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14601/1/src/kudu/master/table_metrics.cc
File src/kudu/master/table_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/14601/1/src/kudu/master/table_metrics.cc@65
PS1, Line 65: bool TableMetrics::TableSupportsLiveRowCount() const {
> Don't we faithfully preserve the last ReportedTabletStatsPB for each tablet
Oops, I forgot that. I only payed attention to the TableMetrics this morning. :(
--
To view, visit http://gerrit.cloudera.org:8080/14601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
Gerrit-Change-Number: 14601
Gerrit-PatchSet: 2
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Fri, 01 Nov 2019 09:08:02 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2986 p2: adjust the 'live row count' metric in master's Web UI
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14601
to look at the new patch set (#2).
Change subject: KUDU-2986 p2: adjust the 'live_row_count' metric in master's
Web UI
..
KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI
The 'live_row_count' metric should be N/A(invalid) in master's Web UI
while there is any tablet which doesn't support live row count.
http://master:8051/metrics?ids=5448c0a8658f467ea373d9044d567011
[
{
"type": "table",
"id": "5448c0a8658f467ea373d9044d567011",
"attributes": {
"table_name": "testtable"
},
"metrics": [
{
"name": "live_row_count",
"value": "N/A"
},
{
"name": "on_disk_size",
"value": 8417243
}
]
}
]
Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
---
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/table_metrics.h
6 files changed, 103 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/14601/2
--
To view, visit http://gerrit.cloudera.org:8080/14601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
Gerrit-Change-Number: 14601
Gerrit-PatchSet: 2
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2986 p2: hide the live row count of table metrics
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14601 ) Change subject: KUDU-2986 p2: hide the live row count of table metrics .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/14601/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/14601/1/src/kudu/master/catalog_manager.cc@5592 PS1, Line 5592: metrics_->HideLiveRowCount(metric_entity_); > When old partitions have been dropped, the metric should not be hidden, rig Yep, you are correct. Thanks for reminding. I have changed to another solution. Please take a look.^_^ http://gerrit.cloudera.org:8080/#/c/14601/1/src/kudu/master/table_metrics.cc File src/kudu/master/table_metrics.cc: http://gerrit.cloudera.org:8080/#/c/14601/1/src/kudu/master/table_metrics.cc@65 PS1, Line 65: METRIC_live_row_count.InstantiateInvalid(entity, 0); > Why this and not on_disk_size->InvalidateEpoch()? It seems that neither InstantiateInvalid() nor InvalidateEpoch() is the best solution, because this design will introduce other problems. For example, if we hide the metric(live_row_count), then we have to ignore the values while it is invisible. But, if we recover this metric(all of the tablets support live row count), how can we ask the tservers to report the stats if there is no any diff on the tserver? Thus, I turn to use InstantiateFunctionGauge instead. All of the stats should be kept in the TableMetrics. -- To view, visit http://gerrit.cloudera.org:8080/14601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533 Gerrit-Change-Number: 14601 Gerrit-PatchSet: 1 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: helifu Gerrit-Comment-Date: Fri, 01 Nov 2019 08:15:51 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2986 p2: hide the live row count of table metrics
helifu has uploaded this change for review. (
http://gerrit.cloudera.org:8080/14601
Change subject: KUDU-2986 p2: hide the live row count of table metrics
..
KUDU-2986 p2: hide the live row count of table metrics
It's easier to hide the 'live row count' metric than to return
an invalid value('N/A') in the master's Web UI while there is
any tablet which doesn't support live row count. And this is
consistent with tserver at the same time.
http://master:8051/metrics?ids=d52a0657c2374b7c8d108196f399d74b
{
"type": "table",
"id": "36c935088fb443139de80c274115170b",
"attributes": {
"table_name": "testtable"
},
"metrics": [
{
"name": "on_disk_size",
"value": 0
}
]
}
Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/master/table_metrics.cc
M src/kudu/master/table_metrics.h
4 files changed, 39 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/14601/1
--
To view, visit http://gerrit.cloudera.org:8080/14601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
Gerrit-Change-Number: 14601
Gerrit-PatchSet: 1
Gerrit-Owner: helifu
[kudu-CR] KUDU-1929 p1: trash is supported while dropping tables
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14566 to look at the new patch set (#3). Change subject: KUDU-1929 p1: trash is supported while dropping tables .. KUDU-1929 p1: trash is supported while dropping tables 1) Add a new field to persist the deletion time for the trashed table/tablets on the master; 2) Add a new boolean variable to represent the trash state for tablets on the tserver; 3) The trash state of the tablet can be synchronized by heartbeat messages (master -> tserver), even if the tserver restarts; 4) The CatalogManagerBgTasks will delete the trash tables that are timeout; 5) Add a new RollbackTable API to roll back the trashed table. Change-Id: I112da81727db6f64026a54311d88e436c34ad6d8 --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/master_proxy_rpc.cc M src/kudu/integration-tests/delete_table-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/master/master_service.h M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tablet_service.h M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/tserver/tserver_admin.proto 18 files changed, 915 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/14566/3 -- To view, visit http://gerrit.cloudera.org:8080/14566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I112da81727db6f64026a54311d88e436c34ad6d8 Gerrit-Change-Number: 14566 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2986 p1: adjust the kudu CLI output from 0 to N/A
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14567
to look at the new patch set (#3).
Change subject: KUDU-2986 p1: adjust the kudu CLI output from 0 to N/A
..
KUDU-2986 p1: adjust the kudu CLI output from 0 to N/A
The `live_row_count` metric value should be invalid(N/A) in
kudu CLI output("kudu table statistics") while there is any
tablet which doesn't support live row count.
Change-Id: I82c47f5ac07cf63d455c0720614c5383bfa88ca3
---
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/table_statistics-internal.h
M src/kudu/master/catalog_manager.cc
M src/kudu/tools/kudu-tool-test.cc
6 files changed, 36 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/14567/3
--
To view, visit http://gerrit.cloudera.org:8080/14567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82c47f5ac07cf63d455c0720614c5383bfa88ca3
Gerrit-Change-Number: 14567
Gerrit-PatchSet: 3
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2986 p1: adjust the output of "table statistics" from 0 -> N/A
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/14567 )
Change subject: KUDU-2986 p1: adjust the output of "table statistics" from 0 ->
N/A
..
Patch Set 2:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/14567/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14567/2//COMMIT_MSG@8
PS2, Line 8:
> Write a meaningful description: what is contained in 'KUDU-2986 p1'.
Done
http://gerrit.cloudera.org:8080/#/c/14567/2/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
http://gerrit.cloudera.org:8080/#/c/14567/2/src/kudu/client/client-test.cc@805
PS2, Line 805: Support
> What supports what? Please write a meaningful sentence here.
Done
http://gerrit.cloudera.org:8080/#/c/14567/2/src/kudu/client/client-test.cc@809
PS2, Line 809: Not support live row count.
> What doesn't supports what? Please write a meaningful sentence here.
Done
http://gerrit.cloudera.org:8080/#/c/14567/2/src/kudu/client/client-test.cc@813
PS2, Line 813: ASSERT_EQ(-1, statistics->live_row_count());
> How do we know FLAGS_live_row_count_for_testing is not -1? Please add
Done
http://gerrit.cloudera.org:8080/#/c/14567/1/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/14567/1/src/kudu/client/client.h@974
PS1, Line 974: /// -1 is returned if the table doesn't support
live_row_count.
> Move this under the @return section.
Done
http://gerrit.cloudera.org:8080/#/c/14567/1/src/kudu/client/table_statistics-internal.h
File src/kudu/client/table_statistics-internal.h:
http://gerrit.cloudera.org:8080/#/c/14567/1/src/kudu/client/table_statistics-internal.h@45
PS1, Line 45: = ""
> While you are at it, please drop this assignment since it's redundant. The
Done
http://gerrit.cloudera.org:8080/#/c/14567/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14567/2/src/kudu/master/catalog_manager.cc@278
PS2, Line 278: support_live_row_count_for_testing
> Please follow the convention for naming flags in this file. This is catalo
Done
http://gerrit.cloudera.org:8080/#/c/14567/2/src/kudu/master/catalog_manager.cc@279
PS2, Line 279: Whether to enable mock support live row count for testing.
> Whether to enable mock live row count statistic for tables. For testing onl
Done
http://gerrit.cloudera.org:8080/#/c/14567/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/14567/2/src/kudu/tools/kudu-tool-test.cc@5574
PS2, Line 5574: TEST_F(ToolTest,
TestGetTableStatisticsLiveRowCountNotSupported) {
> Is there a test to verify functionality of the live row count stats collect
Yep, I think below two test cases are enough:
https://github.com/apache/kudu/blob/0870259456f785c4efdf5826350166d8d6dff5cc/src/kudu/integration-tests/ts_tablet_manager-itest.cc#L699
https://github.com/apache/kudu/blob/0870259456f785c4efdf5826350166d8d6dff5cc/src/kudu/master/master-test.cc#L1778
--
To view, visit http://gerrit.cloudera.org:8080/14567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82c47f5ac07cf63d455c0720614c5383bfa88ca3
Gerrit-Change-Number: 14567
Gerrit-PatchSet: 2
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Wed, 30 Oct 2019 07:33:29 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2986 p1: adjust the output of "table statistics" from 0 -> N/A
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14567 to look at the new patch set (#2). Change subject: KUDU-2986 p1: adjust the output of "table statistics" from 0 -> N/A .. KUDU-2986 p1: adjust the output of "table statistics" from 0 -> N/A Change-Id: I82c47f5ac07cf63d455c0720614c5383bfa88ca3 --- M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/table_statistics-internal.h M src/kudu/master/catalog_manager.cc M src/kudu/tools/kudu-tool-test.cc 6 files changed, 33 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/14567/2 -- To view, visit http://gerrit.cloudera.org:8080/14567 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I82c47f5ac07cf63d455c0720614c5383bfa88ca3 Gerrit-Change-Number: 14567 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2986 p1: adjust the output of "table statistics" from 0 -> N/A
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/14567 )
Change subject: KUDU-2986 p1: adjust the output of "table statistics" from 0 ->
N/A
..
Patch Set 1:
(1 comment)
Do we need to update the java client?
https://github.com/apache/kudu/blob/6ecfe60518803d3963674bf6172f242b5de54780/src/kudu/master/catalog_manager.cc#L2942
https://github.com/apache/kudu/blob/6ecfe60518803d3963674bf6172f242b5de54780/java/kudu-client/src/main/java/org/apache/kudu/client/GetTableStatisticsRequest.java#L72
http://gerrit.cloudera.org:8080/#/c/14567/1/src/kudu/client/table_statistics-internal.h
File src/kudu/client/table_statistics-internal.h:
http://gerrit.cloudera.org:8080/#/c/14567/1/src/kudu/client/table_statistics-internal.h@47
PS1, Line 47: if (live_row_count_) {
: display_string += Substitute("live row count: $0\n",
*live_row_count_);
: } else {
: display_string += Substitute("live row count: N/A\n");
: }
> Rewrite as a ternary:
Done
--
To view, visit http://gerrit.cloudera.org:8080/14567
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82c47f5ac07cf63d455c0720614c5383bfa88ca3
Gerrit-Change-Number: 14567
Gerrit-PatchSet: 1
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Wed, 30 Oct 2019 02:20:20 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2986 p1: adjust the output of "table statistics" from 0 -> N/A
helifu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14567 Change subject: KUDU-2986 p1: adjust the output of "table statistics" from 0 -> N/A .. KUDU-2986 p1: adjust the output of "table statistics" from 0 -> N/A Change-Id: I82c47f5ac07cf63d455c0720614c5383bfa88ca3 --- M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/table_statistics-internal.h 3 files changed, 14 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/67/14567/1 -- To view, visit http://gerrit.cloudera.org:8080/14567 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I82c47f5ac07cf63d455c0720614c5383bfa88ca3 Gerrit-Change-Number: 14567 Gerrit-PatchSet: 1 Gerrit-Owner: helifu
[kudu-CR] KUDU-1929 p1: trash is supported while dropping tables
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14566 to look at the new patch set (#2). Change subject: KUDU-1929 p1: trash is supported while dropping tables .. KUDU-1929 p1: trash is supported while dropping tables 1) Add a new field to persist the deletion time for the trashed table/tablets on the master; 2) Add a new boolean variable to represent the trash state for tablets on the tserver; 3) The trash state of the tablet can be synchronized by heartbeat messages (master -> tserver); 4) The CatalogManagerBgTasks will delete the trash tables that are timeout. Change-Id: I112da81727db6f64026a54311d88e436c34ad6d8 --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/master_proxy_rpc.cc M src/kudu/integration-tests/delete_table-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/master/master_service.h M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tablet_service.h M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/tserver/tserver_admin.proto 18 files changed, 915 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/14566/2 -- To view, visit http://gerrit.cloudera.org:8080/14566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I112da81727db6f64026a54311d88e436c34ad6d8 Gerrit-Change-Number: 14566 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1929 p1: trash is supported while dropping tables
helifu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14566 Change subject: KUDU-1929 p1: trash is supported while dropping tables .. KUDU-1929 p1: trash is supported while dropping tables 1) Add a new field to persist the deletion time for the trashed table/tablets on the master; 2) Add a new boolean variable to represent the trash state for tablets on the tserver; 3) The trash state of the tablet can be synchronized by heartbeat messages (master -> tserver); 4) The CatalogManagerBgTasks will delete the trash tables that are timeout. Change-Id: I112da81727db6f64026a54311d88e436c34ad6d8 --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/master_proxy_rpc.cc M src/kudu/integration-tests/delete_table-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/master/master_service.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tablet_service.h M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/tserver/tserver_admin.proto 19 files changed, 2,164 insertions(+), 1,297 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/14566/1 -- To view, visit http://gerrit.cloudera.org:8080/14566 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I112da81727db6f64026a54311d88e436c34ad6d8 Gerrit-Change-Number: 14566 Gerrit-PatchSet: 1 Gerrit-Owner: helifu
[kudu-CR] [metrics] Hide metric live row count when tablet not support
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14507 ) Change subject: [metrics] Hide metric live_row_count when tablet not support .. Patch Set 12: > (1 comment) ^_^ -- To view, visit http://gerrit.cloudera.org:8080/14507 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a54704d8cbd64a521e65aa3e95bf1a68f7757b7 Gerrit-Change-Number: 14507 Gerrit-PatchSet: 12 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 23 Oct 2019 04:58:14 + Gerrit-HasComments: No
[kudu-CR] [metrics] Hide metric live row count when tablet not support
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14507 ) Change subject: [metrics] Hide metric live_row_count when tablet not support .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/14507/12/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/14507/12/src/kudu/master/catalog_manager.cc@5587 PS12, Line 5587: metrics_->AddTabletNoLiveRowCount(tablet_id); > Under what circumstances will a tablet report include stats but not the on It will insert tablet uuid into std::unordered_set for the tablet which doesn't support LiveRowCount when each report arrives. Thus, I think it will be better to insert only once. And for the uninitialized stat, there is no on disk size. -- To view, visit http://gerrit.cloudera.org:8080/14507 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a54704d8cbd64a521e65aa3e95bf1a68f7757b7 Gerrit-Change-Number: 14507 Gerrit-PatchSet: 12 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 23 Oct 2019 04:12:43 + Gerrit-HasComments: Yes
[kudu-CR] [metrics] Hide metric live row count when tablet not support
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/14507 )
Change subject: [metrics] Hide metric live_row_count when tablet not support
..
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14507/12/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14507/12/src/kudu/master/catalog_manager.cc@5587
PS12, Line 5587: metrics_->AddTabletNoLiveRowCount(tablet_id);
> This could help to alleviate a large number of redundant inserts for the ta
Sorry, should be:
if (!old_stats.has_on_disk_size()) {
metrics_->AddTabletNoLiveRowCount(tablet_id);
}
--
To view, visit http://gerrit.cloudera.org:8080/14507
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a54704d8cbd64a521e65aa3e95bf1a68f7757b7
Gerrit-Change-Number: 14507
Gerrit-PatchSet: 12
Gerrit-Owner: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Wed, 23 Oct 2019 02:03:57 +
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Hide metric live row count when tablet not support
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/14507 )
Change subject: [metrics] Hide metric live_row_count when tablet not support
..
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14507/12/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14507/12/src/kudu/master/catalog_manager.cc@5587
PS12, Line 5587: metrics_->AddTabletNoLiveRowCount(tablet_id);
This could help to alleviate a large number of redundant inserts for the
tablets that doesn't support LiveRowCount:
if (!old_stats.IsInitialized()) {
metrics_->AddTabletNoLiveRowCount(tablet_id);
}
--
To view, visit http://gerrit.cloudera.org:8080/14507
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a54704d8cbd64a521e65aa3e95bf1a68f7757b7
Gerrit-Change-Number: 14507
Gerrit-PatchSet: 12
Gerrit-Owner: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Wed, 23 Oct 2019 02:00:22 +
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Hide metric live row count when tablet not support
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/14507 )
Change subject: [metrics] Hide metric live_row_count when tablet not support
..
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14507/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14507/1//COMMIT_MSG@9
PS1, Line 9: When upgrade an tserver from version older than 1.11, and add some
: partitions for an existing table, then the table will hold some
tablets
: that support live row counting together with some ones that not
support
> Can't we figure that out at runtime using the aggregated input from all tab
All right, thanks for your clarification.
http://gerrit.cloudera.org:8080/#/c/14507/3/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14507/3/src/kudu/master/catalog_manager.cc@5577
PS3, Line 5577: uint64_t old_live_row_count = old_stats.has_live_row_count() ?
old_stats.live_row_count() : 0;
Once Adar told me that the value is 0 even if it is not set in the pb, so they
are the same:
uint64_t old_live_row_count = old_stats.live_row_count();
http://gerrit.cloudera.org:8080/#/c/14507/1/src/kudu/tablet/metadata.proto
File src/kudu/tablet/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/14507/1/src/kudu/tablet/metadata.proto@199
PS1, Line 199: required uint64 on_disk_size = 1;
> Technically both of these should be optional; we should always use optional
the same.
http://gerrit.cloudera.org:8080/#/c/14507/1/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:
http://gerrit.cloudera.org:8080/#/c/14507/1/src/kudu/tablet/tablet.cc@1889
PS1, Line 1889: if (!metadata_->supports_live_row_count()) {
: return Status::NotSupported("This tablet doesn't support
live row counting");
: }
> Should probably be a DCHECK now; it's expected that callers ensure Supports
the same.
http://gerrit.cloudera.org:8080/#/c/14507/1/src/kudu/tablet/tablet_replica.cc
File src/kudu/tablet/tablet_replica.cc:
http://gerrit.cloudera.org:8080/#/c/14507/1/src/kudu/tablet/tablet_replica.cc@812
PS1, Line 812: tablet_->SupportsLiveRowCount()
It't not safe to visit 'tablet_' directly since it will be nullptr.
--
To view, visit http://gerrit.cloudera.org:8080/14507
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a54704d8cbd64a521e65aa3e95bf1a68f7757b7
Gerrit-Change-Number: 14507
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Sat, 19 Oct 2019 13:14:04 +
Gerrit-HasComments: Yes
[kudu-CR] [metrics] Hide metric live row count when tablet not support
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14507 ) Change subject: [metrics] Hide metric live_row_count when tablet not support .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14507/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14507/1//COMMIT_MSG@9 PS1, Line 9: When upgrade an tserver from version older than 1.11, and add some : partitions for an existing table, then the table will hold some tablets : that support live row counting together with some ones that not support The live row counting is base on the newly created tablet, but not for the old ones. So, yes, the aggregated live row counting is not exact at the beginning for the old table which is range partitioned. But, the historical range partitions will be dropped as time going on. That means the number will be correct after all of the old partitions are deleted. If we can't accept this time window, I think we need to add a new flag for SysTablesEntryPB in the master.proto to indicate which table supports this feature and deliver it to the newly create tablets to report stats. What do you think? -- To view, visit http://gerrit.cloudera.org:8080/14507 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a54704d8cbd64a521e65aa3e95bf1a68f7757b7 Gerrit-Change-Number: 14507 Gerrit-PatchSet: 1 Gerrit-Owner: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: helifu Gerrit-Comment-Date: Fri, 18 Oct 2019 12:24:08 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2943: fix the WAL/cmeta term disagreement
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14302 to look at the new patch set (#6). Change subject: KUDU-2943: fix the WAL/cmeta term disagreement .. KUDU-2943: fix the WAL/cmeta term disagreement If we step down a leader tablet, the leader's term will be increased by 1 but not persisted. So it will fail when we reboot the tablet server. Here we use "NO_OP" message to trigger persisting the term for the stepped down tablet. Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 --- M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/integration-tests/raft_consensus_election-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc 4 files changed, 94 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/14302/6 -- To view, visit http://gerrit.cloudera.org:8080/14302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 Gerrit-Change-Number: 14302 Gerrit-PatchSet: 6 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2943: fix the WAL/cmeta term disagreement
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14302 to look at the new patch set (#5). Change subject: KUDU-2943: fix the WAL/cmeta term disagreement .. KUDU-2943: fix the WAL/cmeta term disagreement If we step down a leader tablet, the leader's term will be increased by 1 but not persisted. So it will fail when we reboot the tablet server. Here we use "NO_OP" message to trigger persisting the term for the stepped down tablet. Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 --- M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/integration-tests/raft_consensus_election-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc 4 files changed, 95 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/14302/5 -- To view, visit http://gerrit.cloudera.org:8080/14302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 Gerrit-Change-Number: 14302 Gerrit-PatchSet: 5 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2943: fix the WAL/cmeta term disagreement
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14302 to look at the new patch set (#4). Change subject: KUDU-2943: fix the WAL/cmeta term disagreement .. KUDU-2943: fix the WAL/cmeta term disagreement If we step down a leader tablet, the leader's term will be increased by 1 but not persisted. So it will fail when we reboot the tablet server. Here we use "NO_OP" message to trigger persisting the term for the stepped down tablet. Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 --- M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/integration-tests/raft_consensus_election-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc 4 files changed, 95 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/14302/4 -- To view, visit http://gerrit.cloudera.org:8080/14302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 Gerrit-Change-Number: 14302 Gerrit-PatchSet: 4 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2943: fix the WAL/cmeta term disagreement
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14302 ) Change subject: KUDU-2943: fix the WAL/cmeta term disagreement .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/14302/3/src/kudu/consensus/raft_consensus.cc File src/kudu/consensus/raft_consensus.cc: http://gerrit.cloudera.org:8080/#/c/14302/3/src/kudu/consensus/raft_consensus.cc@563 PS3, Line 563: RETURN_NOT_OK(BecomeReplicaUnlocked()); : last_received_cur_leader_ = MinimumOpId(); > It seems the leader step down is bit hacky by design :) It's some sort of Is it acceptable to trigger the persisting the term by "NO_OP" message? -- To view, visit http://gerrit.cloudera.org:8080/14302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 Gerrit-Change-Number: 14302 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 26 Sep 2019 07:52:33 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2943: fix the WAL/cmeta term disagreement
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14302 ) Change subject: KUDU-2943: fix the WAL/cmeta term disagreement .. Patch Set 3: > (1 comment) > > Doesn't this regress KUDU-2335? This patch effectively reverts > commit f6777db35, no? Sorry, I went the wrong way :( -- To view, visit http://gerrit.cloudera.org:8080/14302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 Gerrit-Change-Number: 14302 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 26 Sep 2019 07:50:01 + Gerrit-HasComments: No
[kudu-CR] KUDU-2943: fix the WAL/cmeta term disagreement
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14302 to look at the new patch set (#3). Change subject: KUDU-2943: fix the WAL/cmeta term disagreement .. KUDU-2943: fix the WAL/cmeta term disagreement If we step down a leader tablet, the leader's term will be increased by 1 but not persisted. So it will fail when we reboot the tablet server. Here we force a step back to avoid this problem while stepping down a leader tablet. Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 --- M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/raft_consensus_election-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc 3 files changed, 77 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/14302/3 -- To view, visit http://gerrit.cloudera.org:8080/14302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 Gerrit-Change-Number: 14302 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2943: fix the WAL/cmeta term disagreement
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14302 ) Change subject: KUDU-2943: fix the WAL/cmeta term disagreement .. Patch Set 2: (1 comment) Thanks, Alexey :) http://gerrit.cloudera.org:8080/#/c/14302/2/src/kudu/consensus/raft_consensus.cc File src/kudu/consensus/raft_consensus.cc: http://gerrit.cloudera.org:8080/#/c/14302/2/src/kudu/consensus/raft_consensus.cc@564 PS2, Line 564: cmeta_->set_current_term(CurrentTermUnlocked() - 1); > So, if current term is X and it's persisted in the meta, then this will set done. -- To view, visit http://gerrit.cloudera.org:8080/14302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 Gerrit-Change-Number: 14302 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 26 Sep 2019 03:34:36 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2943: fix the WAL/cmeta term disagreement
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14302 to look at the new patch set (#2). Change subject: KUDU-2943: fix the WAL/cmeta term disagreement .. KUDU-2943: fix the WAL/cmeta term disagreement If we step down a leader tablet, the leader's term will be increased by 1 but not persisted. So it will fail when we reboot the tablet server. Here we force a step back to avoid this problem while stepping down a leader tablet. Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 --- M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/raft_consensus_election-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc 3 files changed, 76 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/14302/2 -- To view, visit http://gerrit.cloudera.org:8080/14302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 Gerrit-Change-Number: 14302 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2943: fix the WAL/cmeta term disagreement
helifu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14302 Change subject: KUDU-2943: fix the WAL/cmeta term disagreement .. KUDU-2943: fix the WAL/cmeta term disagreement If we step down a leader tablet, the leader's term will be increased by 1 but not persisted. So it will fail whena we reboot the tablet. Here we force a step back while stepping down a leader tablet. Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 --- M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc 2 files changed, 74 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/14302/1 -- To view, visit http://gerrit.cloudera.org:8080/14302 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifc83ce87835f0d9dcb601e4ab602a99843960030 Gerrit-Change-Number: 14302 Gerrit-PatchSet: 1 Gerrit-Owner: helifu
[kudu-CR] KUDU-2952: fix TOCTOU race in reporting stats
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14287 ) Change subject: KUDU-2952: fix TOCTOU race in reporting stats .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14287 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I29fe63cb42c82a08a4811d42b244abeb2fa86bd9 Gerrit-Change-Number: 14287 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Tue, 24 Sep 2019 13:07:36 + Gerrit-HasComments: No
[kudu-CR] [tserver] include ip:port in the tserver name
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14232 to look at the new patch set (#5). Change subject: [tserver] include ip:port in the tserver name .. [tserver] include ip:port in the tserver name Consolidate implementation of ToString() for Master and TabletServer into RpcServer. And include IP:Port info in the TabletServer name at the same time. Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e --- M src/kudu/master/master.cc M src/kudu/master/master.h M src/kudu/server/rpc_server.cc M src/kudu/tserver/tablet_server.cc M src/kudu/tserver/tablet_server.h 5 files changed, 29 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/14232/5 -- To view, visit http://gerrit.cloudera.org:8080/14232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e Gerrit-Change-Number: 14232 Gerrit-PatchSet: 5 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] [tserver] include ip:port in the tserver name
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14232 ) Change subject: [tserver] include ip:port in the tserver name .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/14232/4/src/kudu/tserver/tablet_server.cc File src/kudu/tserver/tablet_server.cc: http://gerrit.cloudera.org:8080/#/c/14232/4/src/kudu/tserver/tablet_server.cc@150 PS4, Line 150: kInitialized == state_ || kStopped == state_ > Maybe, this should be 'state_ != kStopped' or 'state_ == kRunning' instead? Thanks Alexey! -- To view, visit http://gerrit.cloudera.org:8080/14232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e Gerrit-Change-Number: 14232 Gerrit-PatchSet: 4 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 18 Sep 2019 04:17:47 + Gerrit-HasComments: Yes
[kudu-CR] [tserver] include ip:port in the tserver name
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14232 ) Change subject: [tserver] include ip:port in the tserver name .. Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/14232/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14232/3//COMMIT_MSG@7 PS3, Line 7: [tserver] include ip:port in the tserver name > Update this to reflect the new behavior of the commit. Done http://gerrit.cloudera.org:8080/#/c/14232/3/src/kudu/server/rpc_server.h File src/kudu/server/rpc_server.h: http://gerrit.cloudera.org:8080/#/c/14232/3/src/kudu/server/rpc_server.h@82 PS3, Line 82: > Could you restore the ToString() name? ToString() is a strong convention wi Done http://gerrit.cloudera.org:8080/#/c/14232/3/src/kudu/server/rpc_server.cc File src/kudu/server/rpc_server.cc: http://gerrit.cloudera.org:8080/#/c/14232/3/src/kudu/server/rpc_server.cc@111 PS3, Line 111: > You should be able to simplify this using a function from gutil/strings/joi Done -- To view, visit http://gerrit.cloudera.org:8080/14232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e Gerrit-Change-Number: 14232 Gerrit-PatchSet: 4 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 18 Sep 2019 02:08:33 + Gerrit-HasComments: Yes
[kudu-CR] [tserver] include ip:port in the tserver name
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14232 to look at the new patch set (#4). Change subject: [tserver] include ip:port in the tserver name .. [tserver] include ip:port in the tserver name Consolidate implementation of ToString() for Master and TabletServer into RpcServer. And include IP:Port info in the TabletServer name at the same time. Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e --- M src/kudu/master/master.cc M src/kudu/master/master.h M src/kudu/server/rpc_server.cc M src/kudu/tserver/tablet_server.cc M src/kudu/tserver/tablet_server.h 5 files changed, 29 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/14232/4 -- To view, visit http://gerrit.cloudera.org:8080/14232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e Gerrit-Change-Number: 14232 Gerrit-PatchSet: 4 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] [tserver] include ip:port in the tserver name
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14232 to look at the new patch set (#3). Change subject: [tserver] include ip:port in the tserver name .. [tserver] include ip:port in the tserver name Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e --- M src/kudu/master/master.cc M src/kudu/master/master.h M src/kudu/server/rpc_server.cc M src/kudu/server/rpc_server.h M src/kudu/tserver/tablet_server.cc M src/kudu/tserver/tablet_server.h 6 files changed, 36 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/14232/3 -- To view, visit http://gerrit.cloudera.org:8080/14232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e Gerrit-Change-Number: 14232 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2949: fix tablet stats race while ProcessTabletReport
helifu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14245 Change subject: KUDU-2949: fix tablet stats race while ProcessTabletReport .. KUDU-2949: fix tablet stats race while ProcessTabletReport Change-Id: Ic89070bcdb201e2a620efd8fc50041cebd8a7e26 --- M src/kudu/master/catalog_manager.cc 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/14245/1 -- To view, visit http://gerrit.cloudera.org:8080/14245 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic89070bcdb201e2a620efd8fc50041cebd8a7e26 Gerrit-Change-Number: 14245 Gerrit-PatchSet: 1 Gerrit-Owner: helifu
[kudu-CR] [tserver] include ip:port in the tserver name
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/14232 )
Change subject: [tserver] include ip:port in the tserver name
..
Patch Set 2:
> (1 comment)
>
> The TSAN failure is interesting. It's not related to this patch,
> but it is related to one of your previous patches, Lifu.
>
> In ts_tablet_manager-itest.cc:
>
> // 6. Restart the masters.
> {
> for (int i = 0; i < kNumMasters; ++i) {
> int idx = 0;
> ASSERT_OK(cluster_->GetLeaderMasterIndex(&idx));
> master::MiniMaster* mini_master = CHECK_NOTNULL(cluster_->mini_master(idx));
> mini_master->Shutdown();
> SleepFor(MonoDelta::FromMilliseconds(kMaxElectionTime));
> ASSERT_OK(mini_master->Restart());
> // Sometimes the election fails until the node restarts.
> // And the restarted node is elected leader again.
> // So, it is necessary to wait for all tservers to report.
> SleepFor(MonoDelta::FromMilliseconds(FLAGS_heartbeat_interval_ms));
> NO_FATALS(CheckStats(kRowsCount));
> }
> }
>
> I think there needs to be a call to mini_master->WaitForCatalogManagerInit()
> somewhere, perhaps after the Restart(), or after the waiting.
The test case here only cares about the leader master, so I don't think it's
necessary to make sure the restarted master should be online, that means
calling "mini_master->WaitForCatalogManagerInit()" is not required. Besides,
there is evidence that the restarted master had been online before failure,
please look at the full test output from Line2136 to Line2168.
--
To view, visit http://gerrit.cloudera.org:8080/14232
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e
Gerrit-Change-Number: 14232
Gerrit-PatchSet: 2
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Tue, 17 Sep 2019 02:40:07 +
Gerrit-HasComments: No
[kudu-CR] [tserver] include ip:port in the tserver name
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14232 to look at the new patch set (#2). Change subject: [tserver] include ip:port in the tserver name .. [tserver] include ip:port in the tserver name Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e --- M src/kudu/tserver/tablet_server.cc M src/kudu/tserver/tablet_server.h 2 files changed, 19 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/14232/2 -- To view, visit http://gerrit.cloudera.org:8080/14232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e Gerrit-Change-Number: 14232 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [tserver] include ip:port in the tserver name
helifu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14232 Change subject: [tserver] include ip:port in the tserver name .. [tserver] include ip:port in the tserver name Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e --- M src/kudu/tserver/tablet_server.cc M src/kudu/tserver/tablet_server.h 2 files changed, 18 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/14232/1 -- To view, visit http://gerrit.cloudera.org:8080/14232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic6b0d8e29b6cac5a900923b881f0d7a74facbd6e Gerrit-Change-Number: 14232 Gerrit-PatchSet: 1 Gerrit-Owner: helifu
[kudu-CR] [www] support column sorting on /tables page
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/14187 )
Change subject: [www] support column sorting on /tables page
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14187/3/www/kudu.js
File www/kudu.js:
http://gerrit.cloudera.org:8080/#/c/14187/3/www/kudu.js@71
PS3, Line 71: function compareNumericStrings(left, right) {
> What happened to the NaN cases? No longer needed?
I renamed the function name to indicate that it only supports the numeric
strings since it's really ugly and unnecessary to capture all kinds of
exceptions. For example: "019", "-12", "a12", "12a", "9:5". By the way, the
function will return 0 if one of the two is NaN.
Here is an online test for javascript:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt
--
To view, visit http://gerrit.cloudera.org:8080/14187
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibacccd7a98c8bb65cc6736dc7aed285fd8a96ef6
Gerrit-Change-Number: 14187
Gerrit-PatchSet: 3
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Tue, 10 Sep 2019 05:36:58 +
Gerrit-HasComments: Yes
[kudu-CR] [www] support column sorting on /tables page
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14187 to look at the new patch set (#3). Change subject: [www] support column sorting on /tables page .. [www] support column sorting on /tables page Sort the columns of "Table Name", "Create Time", "Last Alter Time". Change-Id: Ibacccd7a98c8bb65cc6736dc7aed285fd8a96ef6 --- M www/kudu.js M www/tables.mustache 2 files changed, 85 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/14187/3 -- To view, visit http://gerrit.cloudera.org:8080/14187 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibacccd7a98c8bb65cc6736dc7aed285fd8a96ef6 Gerrit-Change-Number: 14187 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] [www] support column sorting on /tables page
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14187 to look at the new patch set (#2). Change subject: [www] support column sorting on /tables page .. [www] support column sorting on /tables page Sort the columns of "Table Name", "Create Time", "Last Alter Time". Change-Id: Ibacccd7a98c8bb65cc6736dc7aed285fd8a96ef6 --- M www/kudu.js M www/tables.mustache 2 files changed, 85 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/14187/2 -- To view, visit http://gerrit.cloudera.org:8080/14187 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibacccd7a98c8bb65cc6736dc7aed285fd8a96ef6 Gerrit-Change-Number: 14187 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] [www] support column sorting on /tables page
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/14187 )
Change subject: [www] support column sorting on /tables page
..
Patch Set 1:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/14187/1/www/kudu.js
File www/kudu.js:
http://gerrit.cloudera.org:8080/#/c/14187/1/www/kudu.js@70
PS1, Line 70: function compareTimeUnit(left, right) {
> Doc the function.
Done
http://gerrit.cloudera.org:8080/#/c/14187/1/www/kudu.js@73
PS1, Line 73: return -1;
> Shouldn't this return 1? That is, when sorting time units, shouldn't "valid
I just keep the same logic with Line29. But it seems that your comment is
reasonable, should I change both of them?
http://gerrit.cloudera.org:8080/#/c/14187/1/www/kudu.js@78
PS1, Line 78: }
> nit: what if both left and right are NaN? Does it make sense to compare th
Done
http://gerrit.cloudera.org:8080/#/c/14187/1/www/kudu.js@95
PS1, Line 95: timesSorter
> nit: could you document why simply using stringsSorter() is not an option t
Done
http://gerrit.cloudera.org:8080/#/c/14187/1/www/kudu.js@111
PS1, Line 111: console.log(left, right);
> Do we really want these console.log() statements?
Sorry, a mistake :(
http://gerrit.cloudera.org:8080/#/c/14187/1/www/kudu.js@144
PS1, Line 144: function stringsSorter(left, right) {
> Doc it.
Done
--
To view, visit http://gerrit.cloudera.org:8080/14187
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibacccd7a98c8bb65cc6736dc7aed285fd8a96ef6
Gerrit-Change-Number: 14187
Gerrit-PatchSet: 1
Gerrit-Owner: helifu
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu
Gerrit-Comment-Date: Sun, 08 Sep 2019 08:11:20 +
Gerrit-HasComments: Yes
[kudu-CR] [www] support column sorting on /tables page
helifu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14187 Change subject: [www] support column sorting on /tables page .. [www] support column sorting on /tables page Sort the columns of "Table Name", "Create Time", "Last Alter Time". Change-Id: Ibacccd7a98c8bb65cc6736dc7aed285fd8a96ef6 --- M www/kudu.js M www/tables.mustache 2 files changed, 89 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/14187/1 -- To view, visit http://gerrit.cloudera.org:8080/14187 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ibacccd7a98c8bb65cc6736dc7aed285fd8a96ef6 Gerrit-Change-Number: 14187 Gerrit-PatchSet: 1 Gerrit-Owner: helifu
[kudu-CR] KUDU-2854 short circuit predicates on dictionary-coded columns
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13987 ) Change subject: KUDU-2854 short circuit predicates on dictionary-coded columns .. Patch Set 4: This morning on slack, Todd suggested me to use 'kudu perf loadgen' tool to load data to an existing wide table with ~280+ columns since his benchmark was a customer workload. So, I did some benchmarks today. And the conclusion is that we don't regress that perf gain with current patch since we don't see "DeltaPreparer". Here are the details: ##1.I ran the benchmark with this patch: (a) select * from my_wide_table where c280 = Samples: 2K of event 'cycles', Event count (approx.): 176315474 25.30% rpc worker-6290 kudu-tserver [.] kudu::cfile::BinaryPlainBlockDecoder::CopyNextAndEval(unsigned long*, kudu::ColumnMaterializationContext*, kudu::SelectionVectorView*, kudu 12.46% rpc worker-6290 kudu-tserver [.] kudu::cfile::CFileIterator::PrepareMatchingCodeWords(kudu::ColumnMaterializationContext*) 9.17% rpc worker-6290 kudu-tserver [.] kudu::Slice::compare(kudu::Slice const&) const 3.74% rpc worker-6290 kudu-tserver [.] kudu::tablet::CFileSet::Iterator::CreateColumnIterators(kudu::ScanSpec const*) 2.93% rpc worker-6290 kudu-tserver [.] kudu::cfile::BinaryPlainBlockDecoder::ParseHeader() 2.44% rpc worker-6290 kudu-tserver [.] boost::container::flat_map >, std::less, 1.83% rpc worker-6290 kudu-tserver [.] kudu::tablet::CFileSet::Iterator::GetIteratorStats(std::vector >*) const 1.83% rpc worker-6290 kudu-tserver [.] bshuf_shuffle_bit_eightelem_SSE_avx2 1.83% rpc worker-6290 kudu-tserver [.] operator delete[](void*, std::nothrow_t const&) 1.47% rpc worker-6290 kudu-tserver [.] LZ4_memcpy_using_offset 1.30% rpc reactor-628 [kernel.kallsyms][k] find_busiest_group 1.10% rpc worker-6290 kudu-tserver [.] tcmalloc::CentralFreeList::ReleaseToSpans(void*) 1.10% rpc worker-6290 kudu-tserver [.] tcmalloc::ThreadCache::ReleaseToCentralCache(tcmalloc::ThreadCache::FreeList*, unsigned int, int) 1.10% rpc worker-6290 kudu-tserver [.] kudu::cfile::CFileIterator::Scan(kudu::ColumnMaterializationContext*) 1.10% rpc worker-6290 kudu-tserver [.] kudu::cfile::BinaryDictBlockDecoder::GetFirstRowId() const 1.10% rpc worker-6290 kudu-tserver [.] LZ4_decompress_generic.constprop.3 0.91% rpc worker-6290 libstdc++.so.6.0.21 [.] std::_Hash_bytes(void const*, unsigned long, unsigned long) 0.75% rpc reactor-628 libssl.so.1.0.0 [.] 0x00024667 0.73% rpc worker-6290 kudu-tserver [.] kudu::ArenaBase::Reset() 0.73% rpc worker-6290 kudu-tserver [.] kudu::BitmapChangeBits(unsigned char*, unsigned long, unsigned long, bool) 0.73% rpc worker-6290 libstdc++.so.6.0.21 [.] __cxxabiv1::__si_class_type_info::__do_dyncast(long, __cxxabiv1::__class_type_info::__sub_kind, __cxxabiv1::__class_type_info const*, void 0.73% rpc worker-6290 kudu-tserver [.] kudu::cfile::CFileIterator::~CFileIterator() 0.73% rpc worker-6290 kudu-tserver [.] kudu::cfile::CFileIterator::TrySkipDictCodedBlock(unsigned long*, kudu::SelectionVectorView*, kudu::cfile::BlockDecoder*) const 0.73% rpc worker-6290 kudu-tserver [.] kudu::tablet::DeltaApplier::MaterializeColumn(kudu::ColumnMaterializationContext*) (b) select * from my_wide_table where c280 = Samples: 2K of event 'cycles', Event count (approx.): 233293310 22.17% rpc worker-6290 kudu-tserver [.] kudu::cfile::BinaryPlainBlockDecoder::CopyNextAndEval(unsigned long*, kudu::ColumnMaterializationContext*, kudu::SelectionVectorView*, kudu 10.81% rpc worker-6290 kudu-tserver [.] kudu::cfile::CFileIterator::PrepareMatchingCodeWords(kudu::ColumnMaterializationContext*) 10.25% rpc worker-6290 kudu-tserver [.] kudu::Slice::compare(kudu::Slice const&) const 5.54% rpc worker-6290 kudu-tserver [.] bshuf_shuffle_bit_eightelem_SSE_avx2 4.99% rpc worker-6290 kudu-tserver [.] kudu::cfile::BinaryPlainBlockDecoder::ParseHeader() 2.96% rpc worker-6290 kudu-tserver [.] kudu::tablet::CFileSet::Iterator::CreateColumnIterators(kudu::ScanSpec const*) 2.70% rpc reactor-628 [kernel.kallsyms][k] find_busiest_group 1.66% rpc worker-6290 kudu-tserver [.] bshuf_trans_byte_bitrow_SSE_avx2 1.41% rpc worker-6290 kudu-tserver [.] boost::container::flat_map >, std::less, 1.11% rpc worker-6290 kudu-tserver [.] kudu::tablet::CFileSet::Iterator::PrepareColumn(kudu
[kudu-CR] KUDU-2854 short circuit predicates on dictionary-coded columns
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13987 to look at the new patch set (#4). Change subject: KUDU-2854 short circuit predicates on dictionary-coded columns .. KUDU-2854 short circuit predicates on dictionary-coded columns 1. A dictionary encoding column has no updates in DRS, if there are not entries in the dictionary match the predicate: a) Skip the whole DRS when a flag in the cfile footer is true which indicates that all blocks are dict-coded; b) Skip any front dict-coded blocks without decoding the dictionary words; 2. A dictionary encoding column has updates in DRS, if there are not deltas any more during scanning and the entries in the dictionary doesn't match the predicates: a) Skip all of the remaining dict-coded blocks when the flag in the cfile footer is true; b) Skip the remaining dict-coded blocks without decoding the dictionary words; Change-Id: Id348583cc7d85773e8f32a189f4344d7a36a30b6 --- M src/kudu/cfile/binary_dict_block.h M src/kudu/cfile/cfile-test.cc M src/kudu/cfile/cfile.proto M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/cfile_writer.h M src/kudu/common/column_materialization_context.h M src/kudu/tablet/cfile_set.cc M src/kudu/tablet/delta_applier.cc M src/kudu/tablet/delta_iterator_merger.cc M src/kudu/tablet/delta_iterator_merger.h M src/kudu/tablet/delta_store.cc M src/kudu/tablet/delta_store.h M src/kudu/tablet/deltafile.cc M src/kudu/tablet/deltafile.h M src/kudu/tablet/deltamemstore.cc M src/kudu/tablet/deltamemstore.h M src/kudu/tablet/diskrowset-test.cc M src/kudu/tablet/tablet-test-util.h 20 files changed, 641 insertions(+), 37 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/13987/4 -- To view, visit http://gerrit.cloudera.org:8080/13987 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id348583cc7d85773e8f32a189f4344d7a36a30b6 Gerrit-Change-Number: 13987 Gerrit-PatchSet: 4 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2854 short circuit predicates on dictionary-coded columns
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13987 to look at the new patch set (#3). Change subject: KUDU-2854 short circuit predicates on dictionary-coded columns .. KUDU-2854 short circuit predicates on dictionary-coded columns 1. A dictionary encoding column has no updates in DRS, if there are not entries in the dictionary match the predicate: a) Skip the whole DRS when a flag in the cfile footer is true which indicates that all blocks are dict-coded; b) Skip any front dict-coded blocks without decoding the dictionary words; 2. A dictionary encoding column has updates in DRS, if there are not deltas any more during scanning and the entries in the dictionary doesn't match the predicates: a) Skip all of the remaining dict-coded blocks when the flag in the cfile footer is true; b) Skip the remaining dict-coded blocks without decoding the dictionary words; Change-Id: Id348583cc7d85773e8f32a189f4344d7a36a30b6 --- M src/kudu/cfile/binary_dict_block.h M src/kudu/cfile/cfile-test.cc M src/kudu/cfile/cfile.proto M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/cfile_writer.h M src/kudu/common/column_materialization_context.h M src/kudu/tablet/cfile_set.cc M src/kudu/tablet/delta_applier.cc M src/kudu/tablet/delta_iterator_merger.cc M src/kudu/tablet/delta_iterator_merger.h M src/kudu/tablet/delta_store.cc M src/kudu/tablet/delta_store.h M src/kudu/tablet/deltafile.cc M src/kudu/tablet/deltafile.h M src/kudu/tablet/deltamemstore.cc M src/kudu/tablet/deltamemstore.h M src/kudu/tablet/diskrowset-test.cc M src/kudu/tablet/tablet-test-util.h 20 files changed, 640 insertions(+), 37 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/13987/3 -- To view, visit http://gerrit.cloudera.org:8080/13987 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id348583cc7d85773e8f32a189f4344d7a36a30b6 Gerrit-Change-Number: 13987 Gerrit-PatchSet: 3 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2854 short circuit predicates on dictionary-coded columns
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13987 ) Change subject: KUDU-2854 short circuit predicates on dictionary-coded columns .. Patch Set 2: Sorry for the late update. > In his commit description, Zhang Yao talks about running a benchmark; perhaps > you could reach out to him and ask for more specifics? I asked ZhangYao about the benchmark two weeks ago on WeChat, but she didn't know how to run it either. Maybe I should ask Todd for some help :) -- To view, visit http://gerrit.cloudera.org:8080/13987 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id348583cc7d85773e8f32a189f4344d7a36a30b6 Gerrit-Change-Number: 13987 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Fri, 30 Aug 2019 11:29:40 + Gerrit-HasComments: No
[kudu-CR] KUDU-2854 short circuit predicates on dictionary-coded columns
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13987 to look at the new patch set (#2). Change subject: KUDU-2854 short circuit predicates on dictionary-coded columns .. KUDU-2854 short circuit predicates on dictionary-coded columns 1. A dictionary encoding column has no updates in DRS, if there are not entries in the dictionary match the predicate: a) Skip the whole DRS when a flag in the cfile footer is true which indicates that all blocks are dict-coded; b) Skip any front dict-coded blocks without decoding the dictionary words; 2. A dictionary encoding column has updates in DRS, if there are not deltas any more during scanning and the entries in the dictionary doesn't match the predicates: a) Skip all of the remaining dict-coded blocks when the flag in the cfile footer is true; b) Skip the remaining dict-coded blocks without decoding the dictionary words; Change-Id: Id348583cc7d85773e8f32a189f4344d7a36a30b6 --- M src/kudu/cfile/binary_dict_block.h M src/kudu/cfile/cfile-test.cc M src/kudu/cfile/cfile.proto M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/cfile_writer.h M src/kudu/common/column_materialization_context.h M src/kudu/tablet/cfile_set.cc M src/kudu/tablet/delta_applier.cc M src/kudu/tablet/delta_iterator_merger.cc M src/kudu/tablet/delta_iterator_merger.h M src/kudu/tablet/delta_store.cc M src/kudu/tablet/delta_store.h M src/kudu/tablet/deltafile.cc M src/kudu/tablet/deltafile.h M src/kudu/tablet/deltamemstore.cc M src/kudu/tablet/deltamemstore.h M src/kudu/tablet/diskrowset-test.cc M src/kudu/tablet/tablet-test-util.h 20 files changed, 641 insertions(+), 38 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/13987/2 -- To view, visit http://gerrit.cloudera.org:8080/13987 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id348583cc7d85773e8f32a189f4344d7a36a30b6 Gerrit-Change-Number: 13987 Gerrit-PatchSet: 2 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] [tablet] Fixed the bug of DeltaTracker::CountDeletedRows
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14061 ) Change subject: [tablet] Fixed the bug of DeltaTracker::CountDeletedRows .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/14061/2/src/kudu/tablet/delta_tracker.cc File src/kudu/tablet/delta_tracker.cc: http://gerrit.cloudera.org:8080/#/c/14061/2/src/kudu/tablet/delta_tracker.cc@746 PS2, Line 746: > Is it problematic that the live row count isn't changed atomically as part Yeah, very good case!! And I'm curious about the result: what will happen when the tserver crashes between DMS flushing thread calls CommitRedoDeltaDataBlock and flush the tablet metadata? Meaning, crash between Line746 and Line747. Thanks in advance. http://gerrit.cloudera.org:8080/#/c/14061/4/src/kudu/tablet/rowset_metadata.cc File src/kudu/tablet/rowset_metadata.cc: http://gerrit.cloudera.org:8080/#/c/14061/4/src/kudu/tablet/rowset_metadata.cc@291 PS4, Line 291: std::lock_guard l(lock_); Additional locks to old tables that do not support live row counting. -- To view, visit http://gerrit.cloudera.org:8080/14061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9bb4456123087778c9dc799777c5990938a84fdf Gerrit-Change-Number: 14061 Gerrit-PatchSet: 4 Gerrit-Owner: Yao Xu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yao Xu Gerrit-Reviewer: helifu Gerrit-Comment-Date: Mon, 19 Aug 2019 11:31:23 + Gerrit-HasComments: Yes
[kudu-CR] [tablet] Fixed the bug of DeltaTracker::CountDeletedRows
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/14061 ) Change subject: [tablet] Fixed the bug of DeltaTracker::CountDeletedRows .. Patch Set 2: Code-Review+1 (1 comment) There is a gap between updating "rowset_metadata_" and resetting the "deleted_row_count_" indeed. Yeah, it is a bug! Thanks to Yao Xu. http://gerrit.cloudera.org:8080/#/c/14061/2/src/kudu/tablet/delta_tracker.cc File src/kudu/tablet/delta_tracker.cc: http://gerrit.cloudera.org:8080/#/c/14061/2/src/kudu/tablet/delta_tracker.cc@786 PS2, Line 786: rowset_metadata_->IncrementLiveRows(-old_dms->deleted_row_count()); It seems to be the only way although it looks a bit obtrusive. -- To view, visit http://gerrit.cloudera.org:8080/14061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9bb4456123087778c9dc799777c5990938a84fdf Gerrit-Change-Number: 14061 Gerrit-PatchSet: 2 Gerrit-Owner: Yao Xu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 14 Aug 2019 07:59:36 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2797 p2: the master aggregates tablet statistics
Hello Mike Percy, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13426 to look at the new patch set (#34). Change subject: KUDU-2797 p2: the master aggregates tablet statistics .. KUDU-2797 p2: the master aggregates tablet statistics There are some jiras are talking about metrics: KUDU-1067, KUDU-1373, KUDU-2019, KUDU-2797. In this patch, it makes the following improvements: 1) disk size and live row count of the replicas that are the leadership roles are aggregated on the master server; 2) disk size and live row count of the table are exposed as metrics on the master server; 3) disk size and live row count of the table are exposed on the master server's Web-UI; Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_path_handlers.cc A src/kudu/master/table_metrics.cc A src/kudu/master/table_metrics.h M src/kudu/tablet/local_tablet_writer.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/heartbeater.cc M src/kudu/tserver/heartbeater.h M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/util/metrics.cc M www/table.mustache 22 files changed, 765 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/13426/34 -- To view, visit http://gerrit.cloudera.org:8080/13426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 Gerrit-Change-Number: 13426 Gerrit-PatchSet: 34 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2797 p2: the master aggregates tablet statistics
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13426 ) Change subject: KUDU-2797 p2: the master aggregates tablet statistics .. Patch Set 33: (1 comment) ^_^ http://gerrit.cloudera.org:8080/#/c/13426/33/www/table.mustache File www/table.mustache: http://gerrit.cloudera.org:8080/#/c/13426/33/www/table.mustache@36 PS33, Line 36: Aggregated On-Disk Size(leaders only) > nit: same here, I think it's expected for this page that it's an aggregated Done -- To view, visit http://gerrit.cloudera.org:8080/13426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 Gerrit-Change-Number: 13426 Gerrit-PatchSet: 33 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu Gerrit-Comment-Date: Mon, 12 Aug 2019 07:05:15 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2797 p2: the master aggregates tablet statistics
Hello Mike Percy, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13426 to look at the new patch set (#33). Change subject: KUDU-2797 p2: the master aggregates tablet statistics .. KUDU-2797 p2: the master aggregates tablet statistics There are some jiras are talking about metrics: KUDU-1067, KUDU-1373, KUDU-2019, KUDU-2797. In this patch, it makes the following improvements: 1) disk size and live row count of the replicas that are the leadership roles are aggregated on the master server; 2) disk size and live row count of the table are exposed as metrics on the master server; 3) disk size and live row count of the table are exposed on the master server's Web-UI; Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_path_handlers.cc A src/kudu/master/table_metrics.cc A src/kudu/master/table_metrics.h M src/kudu/tablet/local_tablet_writer.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/heartbeater.cc M src/kudu/tserver/heartbeater.h M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/util/metrics.cc M www/table.mustache 22 files changed, 765 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/13426/33 -- To view, visit http://gerrit.cloudera.org:8080/13426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 Gerrit-Change-Number: 13426 Gerrit-PatchSet: 33 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu
