hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Appy (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-15296) Break out writer and reader from StoreFile
Date Tue, 10 May 2016 01:37:12 GMT

    [ https://issues.apache.org/jira/browse/HBASE-15296?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15277449#comment-15277449
] 

Appy commented on HBASE-15296:
------------------------------

Fully backward compatible patch for branch-1 is here: https://reviews.apache.org/r/47151/.

While makes branch-1 fully backward compatible. it's not forward compatible, for bow. I'll
make a small patch for master branch to finish this off and end things in fully compatible
way. Here's an example of why this mess.

Say there are two functions.
1> void foo(A);
2> A foo();

The refactor moves A to B (Inner StoreFile.Reader class to upper level StoreFileReader class).
And the master change made was:
1> void foo(B);
2> B foo();

To make things backward compatible, the patch changes branch-1 to

add dummy class "A extends B {}"
1> void foo(B);  (This just works due to inheritance)
2> (deprecated) A foo();
2b> B foo2();

Since foo2 was not in master in first patch ,the next change will fix it. It'll be relatively
small.

Why so painful?
- I could have just changed master to make it compatible. But not backporting this change
to branch-1 would make backports pita. I think this one time pain is better than pain in many
future backports (some of may not happen if not for this).
- I learned that such refactoring should not be follow our usual process (master first, then
backport), but should go backward, i.e. figure out earliest branch that will have this change
and then make way towards the master.

> Break out writer and reader from StoreFile
> ------------------------------------------
>
>                 Key: HBASE-15296
>                 URL: https://issues.apache.org/jira/browse/HBASE-15296
>             Project: HBase
>          Issue Type: Improvement
>          Components: regionserver
>            Reporter: Appy
>            Assignee: Appy
>             Fix For: 2.0.0
>
>         Attachments: HBASE-15296-branch-1.1.patch, HBASE-15296-branch-1.2.patch, HBASE-15296-branch-1.patch,
HBASE-15296-master-v2.patch, HBASE-15296-master-v3.patch, HBASE-15296-master-v4.patch, HBASE-15296-master-v5.patch,
HBASE-15296-master.patch
>
>
> StoreFile.java is trending to become a monolithic class, it's ~1800 lines. Would it make
sense to break out reader and writer (~500 lines each) into separate files.
> We are doing so many different things in a single class: comparators, reader, writer,
other stuff; and it hurts readability a lot, to the point that just reading through a piece
of code require scrolling up and down to see which level (reader/writer/base class level)
it belongs to. These small-small things really don't help while trying to understanding the
code. There are good reasons we don't do these often (affects existing patches, needs to be
done for all branches, etc). But this and a few other classes can really use a single iteration
of refactoring to make things a lot better.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message