HBase
  1. HBase
  2. HBASE-2219

stop using code mapping for method names in the RPC

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.20.3
    • Fix Version/s: 0.90.0
    • Component/s: IPC/RPC
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed

      Description

      since we use a sorted mapping of method names -> codes and send that over the wire, even trivial changes, such as adding a new call, become wire-incompatible. This means many features which could easily have gone into a minor update must wait for a major update. Eg: 2066, 1845, etc.

      This will increase on-wire overhead, but the compatibility is worth it I think.

      1. HBASE-2219-2.patch
        12 kB
        ryan rawson
      2. HBASE-2219.patch
        6 kB
        ryan rawson

        Activity

        Hide
        ryan rawson added a comment -

        one test passes, so things dont look horribly broken. I'll run the rest later, but I just wanted to get this up here for discussion...

        Show
        ryan rawson added a comment - one test passes, so things dont look horribly broken. I'll run the rest later, but I just wanted to get this up here for discussion...
        Hide
        ryan rawson added a comment -

        here is a patch that actually works and all tests pass.

        Show
        ryan rawson added a comment - here is a patch that actually works and all tests pass.
        Hide
        ryan rawson added a comment -

        this is a pretty serious change, while it makes all previous RPCs incompatible, it also allows for minor changes to the interface without altering the version number.

        There may be a performance hit, we should consider running some microbenches as well as a larger test.

        Show
        ryan rawson added a comment - this is a pretty serious change, while it makes all previous RPCs incompatible, it also allows for minor changes to the interface without altering the version number. There may be a performance hit, we should consider running some microbenches as well as a larger test.
        Hide
        ryan rawson added a comment -

        I did a PE (with no data, thus testing mostly RPC), and I came up with these:

        with 2199:

        10/02/15 00:41:08 INFO hbase.PerformanceEvaluation: Finished randomRead in 453339ms at offset 0 for 2560000 rows

        without:

        10/02/15 00:56:44 INFO hbase.PerformanceEvaluation: Finished randomRead in 452285ms at offset 0 for 2560000 rows

        Not much of a difference here.

        Show
        ryan rawson added a comment - I did a PE (with no data, thus testing mostly RPC), and I came up with these: with 2199: 10/02/15 00:41:08 INFO hbase.PerformanceEvaluation: Finished randomRead in 453339ms at offset 0 for 2560000 rows without: 10/02/15 00:56:44 INFO hbase.PerformanceEvaluation: Finished randomRead in 452285ms at offset 0 for 2560000 rows Not much of a difference here.
        Hide
        ryan rawson added a comment -

        here is another test:

        without:
        10/02/15 01:06:24 INFO hbase.PerformanceEvaluation: Finished randomRead in 11106ms at offset 0 for 40000 rows

        2199:
        10/02/15 01:09:52 INFO hbase.PerformanceEvaluation: Finished randomRead in 12596ms at offset 0 for 40000 rows

        This is 13% slower, which isnt really that great. Maybe some profiling will help.

        Show
        ryan rawson added a comment - here is another test: without: 10/02/15 01:06:24 INFO hbase.PerformanceEvaluation: Finished randomRead in 11106ms at offset 0 for 40000 rows 2199: 10/02/15 01:09:52 INFO hbase.PerformanceEvaluation: Finished randomRead in 12596ms at offset 0 for 40000 rows This is 13% slower, which isnt really that great. Maybe some profiling will help.
        Hide
        ryan rawson added a comment -

        rerunning, might be facing system variance, eg: chrome messing with the run

        Show
        ryan rawson added a comment - rerunning, might be facing system variance, eg: chrome messing with the run
        Hide
        ryan rawson added a comment -

        ok, doing multiple runs shows quite a bit less variance, and in some cases with this patch is faster .

        2219:
        10/02/15 01:12:34 INFO hbase.PerformanceEvaluation: Finished randomRead in 23423ms at offset 0 for 100000 rows

        10/02/15 01:13:20 INFO hbase.PerformanceEvaluation: Finished randomRead in 22861ms at offset 0 for 100000 rows

        10/02/15 01:14:58 INFO hbase.PerformanceEvaluation: Finished randomRead in 22838ms at offset 0 for 100000 rows

        10/02/15 01:15:39 INFO hbase.PerformanceEvaluation: Finished randomRead in 22851ms at offset 0 for 100000 rows

        without:
        10/02/15 01:19:27 INFO hbase.PerformanceEvaluation: Finished randomRead in 25250ms at offset 0 for 100000 rows

        10/02/15 01:20:03 INFO hbase.PerformanceEvaluation: Finished randomRead in 23326ms at offset 0 for 100000 rows

        10/02/15 01:20:35 INFO hbase.PerformanceEvaluation: Finished randomRead in 23010ms at offset 0 for 100000 rows

        10/02/15 01:21:08 INFO hbase.PerformanceEvaluation: Finished randomRead in 23007ms at offset 0 for 100000 rows

        Show
        ryan rawson added a comment - ok, doing multiple runs shows quite a bit less variance, and in some cases with this patch is faster . 2219: 10/02/15 01:12:34 INFO hbase.PerformanceEvaluation: Finished randomRead in 23423ms at offset 0 for 100000 rows 10/02/15 01:13:20 INFO hbase.PerformanceEvaluation: Finished randomRead in 22861ms at offset 0 for 100000 rows 10/02/15 01:14:58 INFO hbase.PerformanceEvaluation: Finished randomRead in 22838ms at offset 0 for 100000 rows 10/02/15 01:15:39 INFO hbase.PerformanceEvaluation: Finished randomRead in 22851ms at offset 0 for 100000 rows without: 10/02/15 01:19:27 INFO hbase.PerformanceEvaluation: Finished randomRead in 25250ms at offset 0 for 100000 rows 10/02/15 01:20:03 INFO hbase.PerformanceEvaluation: Finished randomRead in 23326ms at offset 0 for 100000 rows 10/02/15 01:20:35 INFO hbase.PerformanceEvaluation: Finished randomRead in 23010ms at offset 0 for 100000 rows 10/02/15 01:21:08 INFO hbase.PerformanceEvaluation: Finished randomRead in 23007ms at offset 0 for 100000 rows
        Hide
        stack added a comment -

        I think the difference is of no consequence; its small. Doing with no content was a good idea. When we going to add this change that breaks RPC?

        Show
        stack added a comment - I think the difference is of no consequence; its small. Doing with no content was a good idea. When we going to add this change that breaks RPC?
        Hide
        Jean-Daniel Cryans added a comment -

        WRT to the patch being sometimes faster, it's really a matter of timing of the splits and region reassignments.

        Show
        Jean-Daniel Cryans added a comment - WRT to the patch being sometimes faster, it's really a matter of timing of the splits and region reassignments.
        Hide
        Jean-Daniel Cryans added a comment -

        Doh forgot you didn't write data, scratch that.

        Show
        Jean-Daniel Cryans added a comment - Doh forgot you didn't write data, scratch that.
        Hide
        stack added a comment -

        Patch looks ok too. I didn't try it.

        Show
        stack added a comment - Patch looks ok too. I didn't try it.
        Hide
        ryan rawson added a comment -

        I am going to commit this to trunk ASAP so it gets into 0.21 sooner than later.

        Now, the question is, do we expect to maintain the 0.20.x branch longer, and if so are we willing to take a 1 time hit to get more expandability. I'm thinking "no".

        Show
        ryan rawson added a comment - I am going to commit this to trunk ASAP so it gets into 0.21 sooner than later. Now, the question is, do we expect to maintain the 0.20.x branch longer, and if so are we willing to take a 1 time hit to get more expandability. I'm thinking "no".
        Hide
        Jean-Daniel Cryans added a comment -

        Bringing in 0.20.4 since we voted +1 on it and assigning Ryan.

        Show
        Jean-Daniel Cryans added a comment - Bringing in 0.20.4 since we voted +1 on it and assigning Ryan.
        Hide
        ryan rawson added a comment -

        I have a pending forward ported patch. Ill commit it soon.

        On Mar 11, 2010 1:58 PM, "Jean-Daniel Cryans (JIRA)" <jira@apache.org>
        wrote:

        [
        https://issues.apache.org/jira/browse/HBASE-2219?page=com.atlassian.jira.plugin.system.issue.
        ..

        Show
        ryan rawson added a comment - I have a pending forward ported patch. Ill commit it soon. On Mar 11, 2010 1:58 PM, "Jean-Daniel Cryans (JIRA)" <jira@apache.org> wrote: [ https://issues.apache.org/jira/browse/HBASE-2219?page=com.atlassian.jira.plugin.system.issue . ..
        Hide
        stack added a comment -

        Committed to branch.

        Show
        stack added a comment - Committed to branch.
        Hide
        ryan rawson added a comment -

        apply to 0.20 branch

        Show
        ryan rawson added a comment - apply to 0.20 branch
        Hide
        ryan rawson added a comment -

        my bad already in there

        Show
        ryan rawson added a comment - my bad already in there
        Hide
        stack added a comment -

        Marking these as fixed against 0.21.0 rather than against 0.20.5.

        Show
        stack added a comment - Marking these as fixed against 0.21.0 rather than against 0.20.5.

          People

          • Assignee:
            ryan rawson
            Reporter:
            ryan rawson
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development