Skip to content
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

[Draft] Improving slow query log payload #37

Closed
wants to merge 1 commit into from

Conversation

rmdmattingly
Copy link

https://issues.apache.org/jira/browse/HBASE-27536

I've loaded the server changes onto a test host and verified the following two states:

  • hbase.regionserver.slowlog.operation.json.enabled == false results in an empty string for operationJson
  • hbase.regionserver.slowlog.operation.json.enabled == true results in an operationJson as expected.

Here's a picture of the entire OnlineLogEntry for a Mutate example:
Screen Shot 2022-12-22 at 1 27 59 PM

And here's a copy of the operationJson for a Scan example:

{
  "filter": "PageFilter 25",
  "startRow": "P\\x00\\x00\\x00",
  "stopRow": "`\\x00\\x00\\x00",
  "batch": -1,
  "cacheBlocks": false,
  "totalColumns": 0,
  "maxResultSize": "4194304",
  "families": {},
  "caching": 2147483647,
  "maxVersions": 1,
  "timeRange": [
    "0",
    "9223372036854775807"
  ]
}

Comment on lines 70 to +76
public SlowLogQueueService(Configuration conf) {
this.isOnlineLogProviderEnabled = conf.getBoolean(HConstants.SLOW_LOG_BUFFER_ENABLED_KEY,
HConstants.DEFAULT_ONLINE_LOG_PROVIDER_ENABLED);
this.slowLogOperationJsonMaxCols = conf.getInt(HConstants.SLOW_LOG_OPERATION_JSON_MAX_COLS,
HConstants.SLOW_LOG_OPERATION_JSON_MAX_COLS_DEFAULT);
this.slowLogOperationJsonEnabled = conf.getBoolean(HConstants.SLOW_LOG_OPERATION_JSON_ENABLED,
HConstants.SLOW_LOG_OPERATION_JSON_ENABLED_DEFAULT);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could consider registering this class with the ConfigurationManager and implementing ConfigurationObserver to make these dynamic, but I omitted that complication from this initial draft. I'm open to input on whether that should make it into our first upstream iteration.

Comment on lines +2170 to +2174
try {
return operation.call().toJSON(maxCols);
} catch (Exception e) {
LOG.warn("Exception when deriving operation JSON", e);
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We introduce the possibility for IOEx on each ProtobufUtil#toX call and on Operation#toJSON. I made this toJson method to unify the suppression of these IOExceptions, but I'd understand if there's pushback on this approach

@rmdmattingly
Copy link
Author

Closing in favor of an upstream PR: apache#4937

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant