Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Per Jim's suggestions on HBase-419, let's move all the store-related files into a subpackage. It should either be o.a.h.h.store or o.a.h.h.regionserver.store. If we push it down another level, I think that we should make sure we never use any of those files outside of o.a.h.h.regionserver.

        Issue Links

          Activity

          Bryan Duxbury created issue -
          Bryan Duxbury made changes -
          Field Original Value New Value
          Priority Major [ 3 ] Minor [ 4 ]
          Summary Move HStore*.java into o.a.h.h.store Move HStore(^Key)*.java into o.a.h.h.store
          Description Per Jim's suggestions on HBase-419, let's move all the store-related files into a subpackage. It should either be o.a.h.h.store or o.a.h.h.regionserver.store. If we push it down another level, I think that we should make sure we never use any of those files outside of o.a.h.h.regionserver.
          Jim Kellerman made changes -
          Assignee Jim Kellerman [ jimk ]
          Jim Kellerman made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Jim Kellerman added a comment -

          HStore.isReference is the only method used outside of HRegionServer. I can move that somewhere else.

          Show
          Jim Kellerman added a comment - HStore.isReference is the only method used outside of HRegionServer. I can move that somewhere else.
          Jim Kellerman made changes -
          Link This issue is blocked by HBASE-468 [ HBASE-468 ]
          Jim Kellerman made changes -
          Link This issue blocks HBASE-469 [ HBASE-469 ]
          Hide
          Jim Kellerman added a comment -

          This is looking ugly. HRegion references a number of methods in HStore and HStoreFile.
          HStore references static methods from HRegion.

          HRegion and HStore are also tightly coupled with HLog.

          We can probably factor out the inner classes of Hregion, HStore and HStoreFile into
          o.a.h.h.regionserver.

          {region,store,storefile(??)}

          , but trying to tease these apart using the current plan will either be a) a ton of work or b) turn out to not be possible.

          Comments, please!

          Show
          Jim Kellerman added a comment - This is looking ugly. HRegion references a number of methods in HStore and HStoreFile. HStore references static methods from HRegion. HRegion and HStore are also tightly coupled with HLog. We can probably factor out the inner classes of Hregion, HStore and HStoreFile into o.a.h.h.regionserver. {region,store,storefile(??)} , but trying to tease these apart using the current plan will either be a) a ton of work or b) turn out to not be possible. Comments, please!
          Hide
          Jim Kellerman added a comment -

          Looking at the issue further, it appears that making subpackages for region, store, etc inner classes is not going to work well. Although many of the inner classes are static, in order for the parent class to access them, too much would have to be made public.

          Just factoring out inner classes (into o.a.h.h.regionserver) and making them package scope would yield the highest containment.

          Show
          Jim Kellerman added a comment - Looking at the issue further, it appears that making subpackages for region, store, etc inner classes is not going to work well. Although many of the inner classes are static, in order for the parent class to access them, too much would have to be made public. Just factoring out inner classes (into o.a.h.h.regionserver) and making them package scope would yield the highest containment.
          Jim Kellerman made changes -
          Link This issue blocks HBASE-469 [ HBASE-469 ]
          Jim Kellerman made changes -
          Status In Progress [ 3 ] Open [ 1 ]
          Jim Kellerman made changes -
          Link This issue is blocked by HBASE-469 [ HBASE-469 ]
          Hide
          Jim Kellerman added a comment - - edited

          On second thought, I am going to put HBASE-467 on the back burner until HBASE-469 is completed. Since the original patch was based on unrefactored code, the changes will be easier to apply now than later.

          Show
          Jim Kellerman added a comment - - edited On second thought, I am going to put HBASE-467 on the back burner until HBASE-469 is completed. Since the original patch was based on unrefactored code, the changes will be easier to apply now than later.
          Hide
          stack added a comment -

          I'm fine w/ all being under o.a.h.h.r rather than under subpackages under o.a.h.h.r – especially if its loads of work. Introducing o.a.h.h.r package is sufficient improvement over what we had previous. Good stuff.

          Show
          stack added a comment - I'm fine w/ all being under o.a.h.h.r rather than under subpackages under o.a.h.h.r – especially if its loads of work. Introducing o.a.h.h.r package is sufficient improvement over what we had previous. Good stuff.
          Hide
          Bryan Duxbury added a comment -

          So let's resolve won't fix?

          Show
          Bryan Duxbury added a comment - So let's resolve won't fix?
          Hide
          Jim Kellerman added a comment -

          There is just too much intermodule coupling to do this properly without breaking encapsulation.

          Show
          Jim Kellerman added a comment - There is just too much intermodule coupling to do this properly without breaking encapsulation.
          Jim Kellerman made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Won't Fix [ 2 ]
          Jim Kellerman made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Jim Kellerman
              Reporter:
              Bryan Duxbury
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development