You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Anonymous Coward (Code Review)" <ge...@cloudera.org> on 2019/11/07 07:41:21 UTC
[kudu-CR] KUDU-2162 Expose stats about scan filters
tdsilva@twilio.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14652
Change subject: KUDU-2162 Expose stats about scan filters
......................................................................
KUDU-2162 Expose stats about scan filters
This patch adds the following resource metrics to scanners.
- bytes read, from disk or cache
- duration scanner was open in nanoseconds
- wall time, cpu time and system time in nanoseconds
These metrics can be used to roughly compare the amount of work done by
scan operations, and could be useful for runtime optimizations in query
planners like Impala or Spark.
Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java
A java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
M src/kudu/client/client-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
M src/kudu/tserver/tserver.proto
8 files changed, 207 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/14652/1
--
To view, visit http://gerrit.cloudera.org:8080/14652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
Gerrit-Change-Number: 14652
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <td...@twilio.com>
[kudu-CR] KUDU-2162 Expose stats about scan filters
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14652
to look at the new patch set (#2).
Change subject: KUDU-2162 Expose stats about scan filters
......................................................................
KUDU-2162 Expose stats about scan filters
This patch adds the following resource metrics to scanners.
- bytes read, from disk or cache
- duration scanner was open in nanoseconds
- wall time, cpu time and system time in nanoseconds
These metrics can be used to roughly compare the amount of work done by
scan operations, and could be useful for runtime optimizations in query
planners like Impala or Spark.
Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java
A java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
M src/kudu/client/client-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
M src/kudu/tserver/tserver.proto
8 files changed, 207 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/14652/2
--
To view, visit http://gerrit.cloudera.org:8080/14652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
Gerrit-Change-Number: 14652
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2162 Expose stats about scan filters
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14652
to look at the new patch set (#3).
Change subject: KUDU-2162 Expose stats about scan filters
......................................................................
KUDU-2162 Expose stats about scan filters
This patch adds the following resource metrics to scanners.
- bytes read, from disk or cache
- duration scanner was open in nanoseconds
- wall time, cpu time and system time in nanoseconds
These metrics can be used to roughly compare the amount of work done by
scan operations, and could be useful for runtime optimizations in query
planners like Impala or Spark.
Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java
A java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
M src/kudu/client/client-test.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_path_handlers.cc
11 files changed, 232 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/14652/3
--
To view, visit http://gerrit.cloudera.org:8080/14652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
Gerrit-Change-Number: 14652
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2162 Expose stats about scan filters
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14652 )
Change subject: KUDU-2162 Expose stats about scan filters
......................................................................
Patch Set 2:
(4 comments)
I haven't reviewed the client-side stuff yet.
http://gerrit.cloudera.org:8080/#/c/14652/1/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/14652/1/src/kudu/tserver/tablet_service.cc@1611
PS1, Line 1611:
Nit: don't need this extra space.
http://gerrit.cloudera.org:8080/#/c/14652/1/src/kudu/tserver/tablet_service.cc@1622
PS1, Line 1622: MonoTime now = MonoTime::Now();
: MonoDelta duration;
: if (scan_descriptor.state == ScanState::kActive) {
: duration = now - scan_descriptor.start_time;
: } else {
: duration = scan_descriptor.last_access_time - scan_descriptor.start_time;
: }
This was copied from tserver_path_handlers.cc; could you decompose it into a ScanDescriptor helper method? Something like:
pair<MonoDelta, MonoDelta> GetScanDurationAndTimeSinceStart() const;
http://gerrit.cloudera.org:8080/#/c/14652/2/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/14652/2/src/kudu/tserver/tablet_service.cc@1613
PS2, Line 1613: const ScanDescriptor& scan_descriptor) {
There are some short-circuit paths that return OK but don't populate the ScanDescriptor, like L2503 for example. How will this function behave if given a default-constructed ScanDescriptor? I think the duration calculation may be messed up. What about the rest?
http://gerrit.cloudera.org:8080/#/c/14652/2/src/kudu/tserver/tablet_service.cc@2692
PS2, Line 2692: *scan_descriptor = scanner.get()->descriptor();
This makes a copy of the ScanDescriptor with each Scan RPC. That's somewhat expensive given the contents of the ScanDescriptor (e.g. projected_columns, predicates, iterator_stats). Given that we only want some quantity metrics (timings and bytes read), perhaps we can avoid the copy? Some ideas:
1. Pass the result collector into the Scanner so that the Scanner can write the metrics into the collector, then use the collector to set up the ResourceMetricsPB when the RPC finishes.
2. Decompose the metrics you care about into a new struct that ScanDescriptor wraps, then get that struct from the Scanner here instead of the full ScanDescriptor.
--
To view, visit http://gerrit.cloudera.org:8080/14652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
Gerrit-Change-Number: 14652
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 07 Nov 2019 22:59:13 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2162 Expose stats about scan filters
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
tdsilva@twilio.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/14652 )
Change subject: KUDU-2162 Expose stats about scan filters
......................................................................
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14652/1/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/14652/1/src/kudu/tserver/tablet_service.cc@1611
PS1, Line 1611:
> Nit: don't need this extra space.
Done
http://gerrit.cloudera.org:8080/#/c/14652/1/src/kudu/tserver/tablet_service.cc@1622
PS1, Line 1622: context->trace()->metrics()->GetMetric(SCANNER_BYTES_READ_METRIC_NAME));
:
: MonoDelta duration = scan_metrics.GetScanDurationAndTimeSinceStart(ScanState::kActive).first;
: metrics->set_duration_nanos(duration.ToNanoseconds());
: metrics->set_cpu_system_nanos(scan_metrics.cpu_times.system);
: metrics->set_cpu_user_nanos(scan_metrics.cpu_times.user);
: m
> This was copied from tserver_path_handlers.cc; could you decompose it into
Done
http://gerrit.cloudera.org:8080/#/c/14652/2/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/14652/2/src/kudu/tserver/tablet_service.cc@1613
PS2, Line 1613: void SetResourceMetrics(rpc::RpcContext* context,
> There are some short-circuit paths that return OK but don't populate the Sc
In the GetScanDurationAndTimeSinceStart() is set to 0 if the start_time or last_access_time is not initialized. The cpu time metrics are also initialized to zero.
http://gerrit.cloudera.org:8080/#/c/14652/2/src/kudu/tserver/tablet_service.cc@2692
PS2, Line 2692: *has_more_results = !req->close_scanner() && iter->HasNext() &&
> This makes a copy of the ScanDescriptor with each Scan RPC. That's somewhat
I moved the metrics from ScanDescriptor into a new ScanMetrics struct. I also added a ScanMetrics field to ScanResultCollector so that we don't have to pass an additional param to HandleNewScanRequest and HandleContinueScanRequest.
--
To view, visit http://gerrit.cloudera.org:8080/14652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
Gerrit-Change-Number: 14652
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 09 Nov 2019 06:43:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2162 Expose stats about scan filters
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14652 )
Change subject: KUDU-2162 Expose stats about scan filters
......................................................................
Patch Set 4:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/scanners.h
File src/kudu/tserver/scanners.h:
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/scanners.h@408
PS4, Line 408: // ScanMetrics contain metrics that will be sent to the client for each batch of rows
Perhaps we should call it ScanRPCMetrics, since:
1) It won't confuse w.r.t. the existing ScannerMetrics
2) It represents quantities for one Scan RPC?
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/scanners.h@409
PS4, Line 409: struct ScanMetrics {
Each unit of indentation should be 2 characters wide, not 4.
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/scanners.h@454
PS4, Line 454: // The cumulative amounts of wall, user cpu, and system cpu time spent on
: // this scanner, in seconds.
This comment isn't as appropriate as it was before. Could you reword it?
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/tablet_service.cc@698
PS4, Line 698: ScanMetrics scan_metrics;
Probably doesn't need to be public. If you make SetResourceMetrics a ScanResultCollector member, this could be private.
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/tserver_path_handlers.cc@531
PS4, Line 531: std::pair<MonoDelta, MonoDelta>
Could replace this with 'auto'.
--
To view, visit http://gerrit.cloudera.org:8080/14652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
Gerrit-Change-Number: 14652
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 09 Nov 2019 22:54:42 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2162 Expose stats about scan filters
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
tdsilva@twilio.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/14652 )
Change subject: KUDU-2162 Expose stats about scan filters
......................................................................
Patch Set 4:
(5 comments)
I pushed a patch with the latest changes to https://gerrit.cloudera.org/c/8375, we can continue with the review there.
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/scanners.h
File src/kudu/tserver/scanners.h:
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/scanners.h@408
PS4, Line 408: // ScanMetrics contain metrics that will be sent to the client for each batch of rows
> Perhaps we should call it ScanRPCMetrics, since:
Done
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/scanners.h@409
PS4, Line 409: struct ScanMetrics {
> Each unit of indentation should be 2 characters wide, not 4.
Done
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/scanners.h@454
PS4, Line 454: // The cumulative amounts of wall, user cpu, and system cpu time spent on
: // this scanner, in seconds.
> This comment isn't as appropriate as it was before. Could you reword it?
Done
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/tablet_service.cc@698
PS4, Line 698: ScanMetrics scan_metrics;
> Probably doesn't need to be public. If you make SetResourceMetrics a ScanRe
Done
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/tserver_path_handlers.cc
File src/kudu/tserver/tserver_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/tserver_path_handlers.cc@531
PS4, Line 531: std::pair<MonoDelta, MonoDelta>
> Could replace this with 'auto'.
Done
--
To view, visit http://gerrit.cloudera.org:8080/14652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
Gerrit-Change-Number: 14652
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 10 Nov 2019 07:25:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2162 Expose stats about scan filters
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14652 )
Change subject: KUDU-2162 Expose stats about scan filters
......................................................................
Patch Set 2:
Just realized that this is a continuation of Will's patch from https://gerrit.cloudera.org/c/8375.
Could you restore that gerrit change and upload a new version of it? That way we can continue the review from where Will left off, and you can explicitly mark all of Todd's feedback as addressed. Mechanically, all you need to do is ensure that the Change-Id is the same in the commit as it was in Will's patch. After that, just push to gerrit and gerrit will allow a second author of the patch.
--
To view, visit http://gerrit.cloudera.org:8080/14652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
Gerrit-Change-Number: 14652
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 07 Nov 2019 23:16:40 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2162 Expose stats about scan filters
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
tdsilva@twilio.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/14652 )
Change subject: KUDU-2162 Expose stats about scan filters
......................................................................
Patch Set 4:
> Uploaded patch set 4.
Sorry for the churn, my local setup is a mac, so the tests were not failing for my locally (due to MonoTime not being initialized).
--
To view, visit http://gerrit.cloudera.org:8080/14652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
Gerrit-Change-Number: 14652
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 09 Nov 2019 20:41:14 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2162 Expose stats about scan filters
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
tdsilva@twilio.com has abandoned this change. ( http://gerrit.cloudera.org:8080/14652 )
Change subject: KUDU-2162 Expose stats about scan filters
......................................................................
Abandoned
Review moved to https://gerrit.cloudera.org/c/8375/
--
To view, visit http://gerrit.cloudera.org:8080/14652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
Gerrit-Change-Number: 14652
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2162 Expose stats about scan filters
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14652
to look at the new patch set (#4).
Change subject: KUDU-2162 Expose stats about scan filters
......................................................................
KUDU-2162 Expose stats about scan filters
This patch adds the following resource metrics to scanners.
- bytes read, from disk or cache
- duration scanner was open in nanoseconds
- wall time, cpu time and system time in nanoseconds
These metrics can be used to roughly compare the amount of work done by
scan operations, and could be useful for runtime optimizations in query
planners like Impala or Spark.
Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java
A java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
M src/kudu/client/client-test.cc
M src/kudu/tserver/scanners.cc
M src/kudu/tserver/scanners.h
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_path_handlers.cc
11 files changed, 232 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/14652/4
--
To view, visit http://gerrit.cloudera.org:8080/14652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
Gerrit-Change-Number: 14652
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2162 Expose stats about scan filters
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14652 )
Change subject: KUDU-2162 Expose stats about scan filters
......................................................................
Patch Set 4:
> > Just realized that this is a continuation of Will's patch from
> > https://gerrit.cloudera.org/c/8375.
> >
> > Could you restore that gerrit change and upload a new version of
> > it? That way we can continue the review from where Will left off,
> > and you can explicitly mark all of Todd's feedback as addressed.
> > Mechanically, all you need to do is ensure that the Change-Id is
> > the same in the commit as it was in Will's patch. After that, just
> > push to gerrit and gerrit will allow a second author of the patch.
>
> Thanks for the review. I tried using the same change id as the patch from https://gerrit.cloudera.org/c/8375 but I get an error when I try to push
>
> ! [remote rejected] HEAD -> refs/for/master (change http://gerrit.cloudera.org:8080/8375 closed)
>
> I think this might be because the patch was abandoned, can you please reopen the change, I don't have permissions.
I reopened it, try again.
--
To view, visit http://gerrit.cloudera.org:8080/14652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
Gerrit-Change-Number: 14652
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 09 Nov 2019 22:47:43 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2162 Expose stats about scan filters
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
tdsilva@twilio.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/14652 )
Change subject: KUDU-2162 Expose stats about scan filters
......................................................................
Patch Set 3:
> Just realized that this is a continuation of Will's patch from
> https://gerrit.cloudera.org/c/8375.
>
> Could you restore that gerrit change and upload a new version of
> it? That way we can continue the review from where Will left off,
> and you can explicitly mark all of Todd's feedback as addressed.
> Mechanically, all you need to do is ensure that the Change-Id is
> the same in the commit as it was in Will's patch. After that, just
> push to gerrit and gerrit will allow a second author of the patch.
I tried using the same change id
> Just realized that this is a continuation of Will's patch from
> https://gerrit.cloudera.org/c/8375.
>
> Could you restore that gerrit change and upload a new version of
> it? That way we can continue the review from where Will left off,
> and you can explicitly mark all of Todd's feedback as addressed.
> Mechanically, all you need to do is ensure that the Change-Id is
> the same in the commit as it was in Will's patch. After that, just
> push to gerrit and gerrit will allow a second author of the patch.
Thanks for the review. I tried using the same change id as the patch from https://gerrit.cloudera.org/c/8375 but I get an error when I try to push
! [remote rejected] HEAD -> refs/for/master (change http://gerrit.cloudera.org:8080/8375 closed)
I think this might be because the patch was abandoned, can you please reopen the change, I don't have permissions.
--
To view, visit http://gerrit.cloudera.org:8080/14652
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55
Gerrit-Change-Number: 14652
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <td...@twilio.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 09 Nov 2019 06:33:37 +0000
Gerrit-HasComments: No