Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.92.0
    • Component/s: IPC/RPC
    • Labels:
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Hide
      Exposes JSON-parseable fingerprint and details for queries that take longer than a configurable threshold time. The exposure is currently to the main regionserver log, along with a (queryTooSlow) tag which allows it to be grepped out and easily aggregated and/or monitored in adminstrator scripts.

      The patch also provides a standard way to extract fingerprint and detail information of interest by requiring each "DatabaseCommand" to provide a fingerprint map and a details map, which will be a superset of the fingerprint.
      Show
      Exposes JSON-parseable fingerprint and details for queries that take longer than a configurable threshold time. The exposure is currently to the main regionserver log, along with a (queryTooSlow) tag which allows it to be grepped out and easily aggregated and/or monitored in adminstrator scripts. The patch also provides a standard way to extract fingerprint and detail information of interest by requiring each "DatabaseCommand" to provide a fingerprint map and a details map, which will be a superset of the fingerprint.

      Description

      Produce log messages for slow queries. The RPC server will decide what is slow based on a configurable "warn response time" parameter. Queries designated as slow will then output a "response too slow" message followed by a fingerprint of the query, and a summary limited in size by another configurable parameter (to limit log spamming).

      1. HBASE-4117.patch
        40 kB
        Riley Patterson
      2. HBASE-4117-v2.patch
        44 kB
        Riley Patterson
      3. HBASE-4117-v3.patch
        44 kB
        Riley Patterson
      4. HBASE-4117-doc.txt
        3 kB
        Riley Patterson

        Activity

        Hide
        Riley Patterson added a comment -

        Exposes JSON-parseable fingerprint and details for queries that take longer than a configurable threshold time. The exposure is currently to the main regionserver log, along with a (queryTooSlow) tag which allows it to be grepped out and easily aggregated and/or monitored in adminstrator scripts.

        The patch also provides a standard way to extract fingerprint and detail information of interest by requiring each "DatabaseCommand" to provide a fingerprint map and a details map, which will be a superset of the fingerprint.

        Show
        Riley Patterson added a comment - Exposes JSON-parseable fingerprint and details for queries that take longer than a configurable threshold time. The exposure is currently to the main regionserver log, along with a (queryTooSlow) tag which allows it to be grepped out and easily aggregated and/or monitored in adminstrator scripts. The patch also provides a standard way to extract fingerprint and detail information of interest by requiring each "DatabaseCommand" to provide a fingerprint map and a details map, which will be a superset of the fingerprint.
        Hide
        Ted Yu added a comment -

        Please combine javadoc in one line:

        +    /** Default value for above param */
        +    private static final int DEFAULT_WARN_RESPONSE_TIME = 1000; // milliseconds
        
        +public abstract class DatabaseCommand {
        

        Maybe we can find a better name for the above class whose purpose is for logging and debugging ?

        Show
        Ted Yu added a comment - Please combine javadoc in one line: + /** Default value for above param */ + private static final int DEFAULT_WARN_RESPONSE_TIME = 1000; // milliseconds + public abstract class DatabaseCommand { Maybe we can find a better name for the above class whose purpose is for logging and debugging ?
        Hide
        stack added a comment -

        How is jettison related to jackson, the json lib we use elsewhere in hbase? It throws events as it trips over the json. Can jackson do this?

        The clause under 'if (processingTime > warnResponseTime) {' could be broken out into a method?

        Should the above test include if logging is DEBUG or more or is thought that this slow logging will be always on?

        You don't want to log something here:

        +            } catch (JSONException e) {
        +              // ignore
        +            }
        

        Could be confusing if I'm expecting logging and its not coming out – would appreciate the failure being logged. There is another instance later in this code block.

        Would 'Operation' be better than 'DatabaseCommand'? Currently this superclass factors into a superclass various kinds of representations of hbase operations – tostring, fingerprint as map, json – etc. Thats fine for now. It could do more with time.

        I like this getFingerprint.

        getDetails seems too generic a name for the method. If you are going to go generic, toMap to go with toString might be better? (Oh, nvm, I see you went one better and made the toString member of the superclass – good).

        Are you removing toString from Delete? Thats probably not good. The toString could use your fingerprinting.

        Shouldn't the methods in Row be added to your new base class? Row Interface should only have 'row' things in it, not getDetails and getFamilyMap.

        A couple of tests would be good where you do some toStringing and toJSON'ing and verify its doing right thing.

        Nice patch Riley. Nice functionality.

        Show
        stack added a comment - How is jettison related to jackson, the json lib we use elsewhere in hbase? It throws events as it trips over the json. Can jackson do this? The clause under 'if (processingTime > warnResponseTime) {' could be broken out into a method? Should the above test include if logging is DEBUG or more or is thought that this slow logging will be always on? You don't want to log something here: + } catch (JSONException e) { + // ignore + } Could be confusing if I'm expecting logging and its not coming out – would appreciate the failure being logged. There is another instance later in this code block. Would 'Operation' be better than 'DatabaseCommand'? Currently this superclass factors into a superclass various kinds of representations of hbase operations – tostring, fingerprint as map, json – etc. Thats fine for now. It could do more with time. I like this getFingerprint. getDetails seems too generic a name for the method. If you are going to go generic, toMap to go with toString might be better? (Oh, nvm, I see you went one better and made the toString member of the superclass – good). Are you removing toString from Delete? Thats probably not good. The toString could use your fingerprinting. Shouldn't the methods in Row be added to your new base class? Row Interface should only have 'row' things in it, not getDetails and getFamilyMap. A couple of tests would be good where you do some toStringing and toJSON'ing and verify its doing right thing. Nice patch Riley. Nice functionality.
        Hide
        Riley Patterson added a comment -

        Jackson does look like it could be a better choice for JSON encoding; I used jettison for consistency with my HLog Pretty Printer (HBASE-3968). It seems like jackson is more actively maintained, and we should probably be consistent throughout the project, so I'll go ahead and change this patch to use jackson as well as patching the HLog Pretty Printer to also use jackson.

        Separating the logging into its own method sounds like a good idea as well. On it.

        The slow logging can be turned off by setting hbase.ipc.warn.response.time very large. I could add an additional check for nonnegativity which would make disabling slow logging a bit more intuitive (set to -1). I do think that this feature should be enabled even when DEBUG logging is not, as it could be useful for DBA's as well as core developers.

        I'm open for suggestions for the name of the super class. Operation sounds fine.

        Show
        Riley Patterson added a comment - Jackson does look like it could be a better choice for JSON encoding; I used jettison for consistency with my HLog Pretty Printer ( HBASE-3968 ). It seems like jackson is more actively maintained, and we should probably be consistent throughout the project, so I'll go ahead and change this patch to use jackson as well as patching the HLog Pretty Printer to also use jackson. Separating the logging into its own method sounds like a good idea as well. On it. The slow logging can be turned off by setting hbase.ipc.warn.response.time very large. I could add an additional check for nonnegativity which would make disabling slow logging a bit more intuitive (set to -1). I do think that this feature should be enabled even when DEBUG logging is not, as it could be useful for DBA's as well as core developers. I'm open for suggestions for the name of the super class. Operation sounds fine.
        Hide
        stack added a comment -

        Thanks Riley.

        Over in hbase-880 when we were talking refactoring, base class there was Operation – e.g. https://issues.apache.org/jira/secure/attachment/12391309/NewCilentAPIProposoal4.gif – in a few proposals and RowOperation in others. You already have a Row interface, so maybe Operation is ok... (I don't see row specifics going on in your Operation instance currently). Good stuff.

        Show
        stack added a comment - Thanks Riley. Over in hbase-880 when we were talking refactoring, base class there was Operation – e.g. https://issues.apache.org/jira/secure/attachment/12391309/NewCilentAPIProposoal4.gif – in a few proposals and RowOperation in others. You already have a Row interface, so maybe Operation is ok... (I don't see row specifics going on in your Operation instance currently). Good stuff.
        Hide
        Andrew Purtell added a comment -

        I'm not suggesting turning down good work.

        However I renew my objection to the result of adding this feature as is. We end up with a hodgepodge of methods for gathering status information or taking administrative action. Do I use the shell? Do I grab some JSON at some path on the server? (And at more than one path?) Do I grep the logs? This is not good design. Why can't this information be made retrievable via the Java admin API via RPC (and therefore the shell can manipulate it)? Why can't we have a common interface that lets us expose this type of information by RPC or by servlet or by logging?

        Show
        Andrew Purtell added a comment - I'm not suggesting turning down good work. However I renew my objection to the result of adding this feature as is. We end up with a hodgepodge of methods for gathering status information or taking administrative action. Do I use the shell? Do I grab some JSON at some path on the server? (And at more than one path?) Do I grep the logs? This is not good design. Why can't this information be made retrievable via the Java admin API via RPC (and therefore the shell can manipulate it)? Why can't we have a common interface that lets us expose this type of information by RPC or by servlet or by logging?
        Hide
        Riley Patterson added a comment -

        This isn't really meant to enable gathering of status information. It is a bunch of historical information which is highly related to other information in the main log. Without the context of log events regarding compactions, region reassignments, etc. that we gain by placing it in the main log, we lose most of the utility. It can also provide context to other log events that may have been caused by RPC handlers getting locked up on slow queries.

        We could additionally log this information somewhere in a monitor in memory or in its own separate log and open this up to RPC access, but it would duplicate a lot of historical information which would likely see infrequent use. I don't think there's any more reason to do this for this particular feature than for most other events logged in our main log file.

        Note that I'm also finishing up work on an initial implementation of HBASE-4057, which deals directly with status information for the calls currently being serviced by RPC handlers and enables exposure of the contents of getFingerprint and getDetails in a more general way. This information resides in the TaskMonitor singleton, which can then be exposed any way you like. I think this design makes a lot of sense; historical information goes in the main log, while current status information can be queried from a clean interface and exposed any way you like.

        Show
        Riley Patterson added a comment - This isn't really meant to enable gathering of status information. It is a bunch of historical information which is highly related to other information in the main log. Without the context of log events regarding compactions, region reassignments, etc. that we gain by placing it in the main log, we lose most of the utility. It can also provide context to other log events that may have been caused by RPC handlers getting locked up on slow queries. We could additionally log this information somewhere in a monitor in memory or in its own separate log and open this up to RPC access, but it would duplicate a lot of historical information which would likely see infrequent use. I don't think there's any more reason to do this for this particular feature than for most other events logged in our main log file. Note that I'm also finishing up work on an initial implementation of HBASE-4057 , which deals directly with status information for the calls currently being serviced by RPC handlers and enables exposure of the contents of getFingerprint and getDetails in a more general way. This information resides in the TaskMonitor singleton, which can then be exposed any way you like. I think this design makes a lot of sense; historical information goes in the main log, while current status information can be queried from a clean interface and exposed any way you like.
        Hide
        Andrew Purtell added a comment -

        This information resides in the TaskMonitor singleton, which can then be exposed any way you like. I think this design makes a lot of sense; historical information goes in the main log, while current status information can be queried from a clean interface and exposed any way you like.

        Fair enough. I stand corrected / mollified.

        Show
        Andrew Purtell added a comment - This information resides in the TaskMonitor singleton, which can then be exposed any way you like. I think this design makes a lot of sense; historical information goes in the main log, while current status information can be queried from a clean interface and exposed any way you like. Fair enough. I stand corrected / mollified.
        Hide
        Gary Helmling added a comment -

        Riley, would you mind putting this up on reviews.apache.org? This is a pretty big change to read through as a patch. Thanks!

        Show
        Gary Helmling added a comment - Riley, would you mind putting this up on reviews.apache.org? This is a pretty big change to read through as a patch. Thanks!
        Hide
        stack added a comment -

        @Andrew I appreciate your showing up in your architect hat flashing the red card. Please stay on the prowl and flag the violations.

        In this case, on review, I think we are talking honest logging (logging already has a system that facilitates redirection and feeding events to other than files that live under HBASE_LOG_DIR – which reminds me, I need to go back to Riley and make sure that these events CAN be directed to other than the .log file cleanly). As Riley says, these logs at an extreme will likely make most sense if shown in context amidst the logging of what else was going on on the server.

        The patch also includes nice refactoring of base classes – Scan, Get, etc. – to make it so they can dump out a self-description (either a fingerprint/synopsis, or a full catalog of the payload being carried) in text or json that looks like it can be passed to whatever asks for the info (log or metrics or...)

        We are still missing the general theory on how to pass hbase emissions but I think this patch helps move us closer to a general system much more than it adds distraction.

        Show
        stack added a comment - @Andrew I appreciate your showing up in your architect hat flashing the red card. Please stay on the prowl and flag the violations. In this case, on review, I think we are talking honest logging (logging already has a system that facilitates redirection and feeding events to other than files that live under HBASE_LOG_DIR – which reminds me, I need to go back to Riley and make sure that these events CAN be directed to other than the .log file cleanly). As Riley says, these logs at an extreme will likely make most sense if shown in context amidst the logging of what else was going on on the server. The patch also includes nice refactoring of base classes – Scan, Get, etc. – to make it so they can dump out a self-description (either a fingerprint/synopsis, or a full catalog of the payload being carried) in text or json that looks like it can be passed to whatever asks for the info (log or metrics or...) We are still missing the general theory on how to pass hbase emissions but I think this patch helps move us closer to a general system much more than it adds distraction.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1621/
        -----------------------------------------------------------

        (Updated 2011-08-22 23:25:12.626245)

        Review request for hbase.

        Changes
        -------

        Addressed stack's comments from last week, cleaned up code, standardized many fields in the JSON output.

        Summary
        -------

        Exposes JSON-parseable fingerprint and details for queries that take longer than a configurable threshold time. The exposure is currently to the main regionserver log, along with a (operationTooSlow) tag which allows it to be grepped out and easily aggregated and/or monitored in administrator scripts.

        The patch also provides a standard way to extract fingerprint and detail information of interest by requiring each "Operation" to provide a fingerprint map and a details map, which will be a superset of the fingerprint.

        This addresses bug HBASE-4117.
        https://issues.apache.org/jira/browse/HBASE-4117

        Diffs


        /src/main/java/org/apache/hadoop/hbase/KeyValue.java 1160468
        /src/main/java/org/apache/hadoop/hbase/client/Delete.java 1160468
        /src/main/java/org/apache/hadoop/hbase/client/Get.java 1160468
        /src/main/java/org/apache/hadoop/hbase/client/MultiAction.java 1160468
        /src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1160468
        /src/main/java/org/apache/hadoop/hbase/client/Operation.java PRE-CREATION
        /src/main/java/org/apache/hadoop/hbase/client/Put.java 1160468
        /src/main/java/org/apache/hadoop/hbase/client/Scan.java 1160468
        /src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1160468
        /src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 1160468
        /src/test/java/org/apache/hadoop/hbase/client/TestOperation.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1621/diff

        Testing
        -------

        Monitored get and put latency with and without the patch using reasonable hbase.ipc.warn.response.time settings (1000ms, 2000ms). Performance was not noticeably impacted.

        Thanks,

        Riley

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1621/ ----------------------------------------------------------- (Updated 2011-08-22 23:25:12.626245) Review request for hbase. Changes ------- Addressed stack's comments from last week, cleaned up code, standardized many fields in the JSON output. Summary ------- Exposes JSON-parseable fingerprint and details for queries that take longer than a configurable threshold time. The exposure is currently to the main regionserver log, along with a (operationTooSlow) tag which allows it to be grepped out and easily aggregated and/or monitored in administrator scripts. The patch also provides a standard way to extract fingerprint and detail information of interest by requiring each "Operation" to provide a fingerprint map and a details map, which will be a superset of the fingerprint. This addresses bug HBASE-4117 . https://issues.apache.org/jira/browse/HBASE-4117 Diffs /src/main/java/org/apache/hadoop/hbase/KeyValue.java 1160468 /src/main/java/org/apache/hadoop/hbase/client/Delete.java 1160468 /src/main/java/org/apache/hadoop/hbase/client/Get.java 1160468 /src/main/java/org/apache/hadoop/hbase/client/MultiAction.java 1160468 /src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1160468 /src/main/java/org/apache/hadoop/hbase/client/Operation.java PRE-CREATION /src/main/java/org/apache/hadoop/hbase/client/Put.java 1160468 /src/main/java/org/apache/hadoop/hbase/client/Scan.java 1160468 /src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1160468 /src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 1160468 /src/test/java/org/apache/hadoop/hbase/client/TestOperation.java PRE-CREATION Diff: https://reviews.apache.org/r/1621/diff Testing ------- Monitored get and put latency with and without the patch using reasonable hbase.ipc.warn.response.time settings (1000ms, 2000ms). Performance was not noticeably impacted. Thanks, Riley
        Hide
        Riley Patterson added a comment -

        Addressed many of the comments and suggestions from this thread.

        Also posted on: https://reviews.apache.org/r/1621/

        Show
        Riley Patterson added a comment - Addressed many of the comments and suggestions from this thread. Also posted on: https://reviews.apache.org/r/1621/
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1621/
        -----------------------------------------------------------

        (Updated 2011-08-26 04:00:49.869021)

        Review request for hbase.

        Changes
        -------

        Several code-cleanup related changes.

        Summary
        -------

        Exposes JSON-parseable fingerprint and details for queries that take longer than a configurable threshold time. The exposure is currently to the main regionserver log, along with a (operationTooSlow) tag which allows it to be grepped out and easily aggregated and/or monitored in administrator scripts.

        The patch also provides a standard way to extract fingerprint and detail information of interest by requiring each "Operation" to provide a fingerprint map and a details map, which will be a superset of the fingerprint.

        This addresses bug HBASE-4117.
        https://issues.apache.org/jira/browse/HBASE-4117

        Diffs (updated)


        /src/main/java/org/apache/hadoop/hbase/KeyValue.java 1161979
        /src/main/java/org/apache/hadoop/hbase/client/Delete.java 1161979
        /src/main/java/org/apache/hadoop/hbase/client/Get.java 1161979
        /src/main/java/org/apache/hadoop/hbase/client/MultiAction.java 1161979
        /src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1161979
        /src/main/java/org/apache/hadoop/hbase/client/Operation.java PRE-CREATION
        /src/main/java/org/apache/hadoop/hbase/client/Put.java 1161979
        /src/main/java/org/apache/hadoop/hbase/client/Scan.java 1161979
        /src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1161979
        /src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 1161979
        /src/test/java/org/apache/hadoop/hbase/client/TestOperation.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1621/diff

        Testing
        -------

        Monitored get and put latency with and without the patch using reasonable hbase.ipc.warn.response.time settings (1000ms, 2000ms). Performance was not noticeably impacted.

        Thanks,

        Riley

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1621/ ----------------------------------------------------------- (Updated 2011-08-26 04:00:49.869021) Review request for hbase. Changes ------- Several code-cleanup related changes. Summary ------- Exposes JSON-parseable fingerprint and details for queries that take longer than a configurable threshold time. The exposure is currently to the main regionserver log, along with a (operationTooSlow) tag which allows it to be grepped out and easily aggregated and/or monitored in administrator scripts. The patch also provides a standard way to extract fingerprint and detail information of interest by requiring each "Operation" to provide a fingerprint map and a details map, which will be a superset of the fingerprint. This addresses bug HBASE-4117 . https://issues.apache.org/jira/browse/HBASE-4117 Diffs (updated) /src/main/java/org/apache/hadoop/hbase/KeyValue.java 1161979 /src/main/java/org/apache/hadoop/hbase/client/Delete.java 1161979 /src/main/java/org/apache/hadoop/hbase/client/Get.java 1161979 /src/main/java/org/apache/hadoop/hbase/client/MultiAction.java 1161979 /src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1161979 /src/main/java/org/apache/hadoop/hbase/client/Operation.java PRE-CREATION /src/main/java/org/apache/hadoop/hbase/client/Put.java 1161979 /src/main/java/org/apache/hadoop/hbase/client/Scan.java 1161979 /src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1161979 /src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 1161979 /src/test/java/org/apache/hadoop/hbase/client/TestOperation.java PRE-CREATION Diff: https://reviews.apache.org/r/1621/diff Testing ------- Monitored get and put latency with and without the patch using reasonable hbase.ipc.warn.response.time settings (1000ms, 2000ms). Performance was not noticeably impacted. Thanks, Riley
        Hide
        Riley Patterson added a comment -

        Small code-cleanup changes.

        Show
        Riley Patterson added a comment - Small code-cleanup changes.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1621/#review1669
        -----------------------------------------------------------

        Ship it!

        /src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
        <https://reviews.apache.org/r/1621/#comment3771>

        Why remove this stuff Riley? Seems useful? Or, looks like you moved it. What you thinking? Move it out of the generic into the specific implementation engine?

        /src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java
        <https://reviews.apache.org/r/1621/#comment3772>

        nice

        Doing check for both too large and too slow

        I suppose this answers my previous question.

        • Michael

        On 2011-08-26 04:00:49, Riley Patterson wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1621/

        -----------------------------------------------------------

        (Updated 2011-08-26 04:00:49)

        Review request for hbase.

        Summary

        -------

        Exposes JSON-parseable fingerprint and details for queries that take longer than a configurable threshold time. The exposure is currently to the main regionserver log, along with a (operationTooSlow) tag which allows it to be grepped out and easily aggregated and/or monitored in administrator scripts.

        The patch also provides a standard way to extract fingerprint and detail information of interest by requiring each "Operation" to provide a fingerprint map and a details map, which will be a superset of the fingerprint.

        This addresses bug HBASE-4117.

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

        Diffs

        -----

        /src/main/java/org/apache/hadoop/hbase/KeyValue.java 1161979

        /src/main/java/org/apache/hadoop/hbase/client/Delete.java 1161979

        /src/main/java/org/apache/hadoop/hbase/client/Get.java 1161979

        /src/main/java/org/apache/hadoop/hbase/client/MultiAction.java 1161979

        /src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1161979

        /src/main/java/org/apache/hadoop/hbase/client/Operation.java PRE-CREATION

        /src/main/java/org/apache/hadoop/hbase/client/Put.java 1161979

        /src/main/java/org/apache/hadoop/hbase/client/Scan.java 1161979

        /src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1161979

        /src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 1161979

        /src/test/java/org/apache/hadoop/hbase/client/TestOperation.java PRE-CREATION

        Diff: https://reviews.apache.org/r/1621/diff

        Testing

        -------

        Monitored get and put latency with and without the patch using reasonable hbase.ipc.warn.response.time settings (1000ms, 2000ms). Performance was not noticeably impacted.

        Thanks,

        Riley

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1621/#review1669 ----------------------------------------------------------- Ship it! /src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java < https://reviews.apache.org/r/1621/#comment3771 > Why remove this stuff Riley? Seems useful? Or, looks like you moved it. What you thinking? Move it out of the generic into the specific implementation engine? /src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java < https://reviews.apache.org/r/1621/#comment3772 > nice Doing check for both too large and too slow I suppose this answers my previous question. Michael On 2011-08-26 04:00:49, Riley Patterson wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1621/ ----------------------------------------------------------- (Updated 2011-08-26 04:00:49) Review request for hbase. Summary ------- Exposes JSON-parseable fingerprint and details for queries that take longer than a configurable threshold time. The exposure is currently to the main regionserver log, along with a (operationTooSlow) tag which allows it to be grepped out and easily aggregated and/or monitored in administrator scripts. The patch also provides a standard way to extract fingerprint and detail information of interest by requiring each "Operation" to provide a fingerprint map and a details map, which will be a superset of the fingerprint. This addresses bug HBASE-4117 . https://issues.apache.org/jira/browse/HBASE-4117 Diffs ----- /src/main/java/org/apache/hadoop/hbase/KeyValue.java 1161979 /src/main/java/org/apache/hadoop/hbase/client/Delete.java 1161979 /src/main/java/org/apache/hadoop/hbase/client/Get.java 1161979 /src/main/java/org/apache/hadoop/hbase/client/MultiAction.java 1161979 /src/main/java/org/apache/hadoop/hbase/client/MultiPut.java 1161979 /src/main/java/org/apache/hadoop/hbase/client/Operation.java PRE-CREATION /src/main/java/org/apache/hadoop/hbase/client/Put.java 1161979 /src/main/java/org/apache/hadoop/hbase/client/Scan.java 1161979 /src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1161979 /src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 1161979 /src/test/java/org/apache/hadoop/hbase/client/TestOperation.java PRE-CREATION Diff: https://reviews.apache.org/r/1621/diff Testing ------- Monitored get and put latency with and without the patch using reasonable hbase.ipc.warn.response.time settings (1000ms, 2000ms). Performance was not noticeably impacted. Thanks, Riley
        Hide
        stack added a comment -

        Nice one Riley. Committed to TRUNK (For future, be careful on the white space.. I cleaned it up before commit).

        Show
        stack added a comment - Nice one Riley. Committed to TRUNK (For future, be careful on the white space.. I cleaned it up before commit).
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2151 (See https://builds.apache.org/job/HBase-TRUNK/2151/)
        HBASE-4117 Slow Query Log and Client Operation Fingerprints

        stack :
        Files :

        • /hbase/trunk/CHANGES.txt
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/KeyValue.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiAction.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestOperation.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2151 (See https://builds.apache.org/job/HBase-TRUNK/2151/ ) HBASE-4117 Slow Query Log and Client Operation Fingerprints stack : Files : /hbase/trunk/CHANGES.txt /hbase/trunk/src/main/java/org/apache/hadoop/hbase/KeyValue.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Delete.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Get.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiAction.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MultiPut.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Operation.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Put.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/Scan.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestOperation.java
        Hide
        Riley Patterson added a comment -

        A basic, user-manual type of document describing the feature, its use, its configuration, and grokking its output.

        Show
        Riley Patterson added a comment - A basic, user-manual type of document describing the feature, its use, its configuration, and grokking its output.
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2645 (See https://builds.apache.org/job/HBase-TRUNK/2645/)
        Add Riley's slow query doc from hbase-4117

        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2645 (See https://builds.apache.org/job/HBase-TRUNK/2645/ ) Add Riley's slow query doc from hbase-4117
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK-security #86 (See https://builds.apache.org/job/HBase-TRUNK-security/86/)
        Add Riley's slow query doc from hbase-4117

        Show
        Hudson added a comment - Integrated in HBase-TRUNK-security #86 (See https://builds.apache.org/job/HBase-TRUNK-security/86/ ) Add Riley's slow query doc from hbase-4117

          People

          • Assignee:
            Riley Patterson
            Reporter:
            Riley Patterson
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development