-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hrpc: enable ScanMetrics #254
Conversation
74ea11a
to
1c3410a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few comments but I also wonder if adding the scan metrics to the hrpc.Results is the right choice here. It seems that we will be allocating a lot of small struct on the heap to hold these result and most likely the user of this feature wants to know the overall scan metrics at the end so they will have to sum the various result in their own code.
Would it make more sense to hold the scan metrics in the scanner struct itself, accumulating the result as we fetch and add a method to the scanner to returns the end metrics. And also doing that while keepting the scan metrics as map[string]int64 such that there is no dependencies with the HBase version at all (if a new version of HBase introduce a new scan metrics name, or change one, we don't need to update GoHBase accordintly).
What are your thoughts on this?
I echo Thibault's feedback here that always passing around the scan metrics along with the batch of results makes the code more complex while also making the scan metrics harder to use from the caller's point of view. Not sure we should keep them in a generic |
46d7b0d
to
643c294
Compare
643c294
to
875e7ef
Compare
Enable tracking scan metrics in the ScanResponse. Clients can access the metrics via calls to scanner.GetScanMetrics() Support for HBase versions < 2.6.0 where ScanMetrics provides ROWS_SCANNED and ROWS_FILTERED metrics.
875e7ef
to
5f0e62d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚢
Enable tracking scan metrics in the ScanResponse. Clients can access the metrics via calls to scanner.Next()
Support for HBase versions < 2.6.0 where ScanMetrics provides ROWS_SCANNED and ROWS_FILTERED metrics.