[
https://issues.apache.org/jira/browse/HBASE-10378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13885058#comment-13885058
]
stack commented on HBASE-10378:
-------------------------------
I like the doing away with the awful HLog name replacing it w/ WAL.
I suppose we should remove NamespaceUpgrade.java now we are past out 0.96. But
that is not for this issue.
This is a bit odd, returning a 'Service' when we asked for a 'Log':
- public HLog getLog() {
+ public WALService getLog() {
return this.log;
}
Similar here, final WALService wal, in that the parameter name should be
walService, not just wal... and here 'WALService log'
Here in the comment it says:
+ * need to explicitly close the {@link WALService} it created too;
... but I'd think we'd start and stop a Service, not 'close' it. What you
think?
It shouldn't be '{@link WALService} file. ' a file here... how WALService is
implemented should not be a concern of the user (whether it uses files or not).
This is a 'nit'.
Yeah, you 'stop' a service I'd say: '+ * The {@link WALService} for the
created region needs to be closed explicitly.'
Fix this:
final WALService hlog,
This should be cleaned up:
+ WALService effectiveHLog = hlog;
something like '+ WALService effectiveWALService = walService;'
Fix ' final WALService hlog)'
+ protected volatile WALService hlogForMeta; should be 'walServiceForMeta'.
+ private WALService getMetaWAL() throws IOException { should be
getMetaWALService
+ protected WALService instantiateHLog( should be instantiateWALService...
I think you get the point.
I skipped forward to the Interfaces:
It is a pity that rollWriter has to be let outside of WALService but there is
probably little to do about it.
stop rather than 'close'. Does it need a 'start'?
+ // TODO: Do we need all these versions of sync?
+ void hsync() throws IOException;
+
+ void hflush() throws IOException;
+
+ void sync() throws IOException;
+
+ void sync(long txid) throws IOException;
?
Does this have to be in the WALService -- + WALCoprocessorHost
getCoprocessorHost();?
Otherwise, the WALService looks nice and clean.
These belong in WALService instead?
+ /**
+ * @return the total number of WAL files (including rolled WALs).
+ */
+ int getNumLogFiles();
+
+ /**
+ * returns the number of rolled WAL files.
+ */
+ int getNumRolledLogFiles();
+
+ /**
+ * @return the path of the current WAL file.
+ */
+ Path getCurrentFileName();
+
+ /**
+ * Get LowReplication-Roller status
+ * @return lowReplicationRollEnabled
+ */
+ boolean isLowReplicationRollEnabled();
We should get rid of this:
+ // TODO: Remove this Writable.
+ class Entry implements Writable {
Should just be the pb. But another patch.
Otherwise WAL looks good.
> Divide HLog interface into User and Implementor specific interfaces
> -------------------------------------------------------------------
>
> Key: HBASE-10378
> URL: https://issues.apache.org/jira/browse/HBASE-10378
> Project: HBase
> Issue Type: Sub-task
> Components: wal
> Reporter: Himanshu Vashishtha
> Attachments: 10378-1.patch, 10378-2.patch
>
>
> HBASE-5937 introduces the HLog interface as a first step to support multiple
> WAL implementations. This interface is a good start, but has some
> limitations/drawbacks in its current state, such as:
> 1) There is no clear distinction b/w User and Implementor APIs, and it
> provides APIs both for WAL users (append, sync, etc) and also WAL
> implementors (Reader/Writer interfaces, etc). There are APIs which are very
> much implementation specific (getFileNum, etc) and a user such as a
> RegionServer shouldn't know about it.
> 2) There are about 14 methods in FSHLog which are not present in HLog
> interface but are used at several places in the unit test code. These tests
> typecast HLog to FSHLog, which makes it very difficult to test multiple WAL
> implementations without doing some ugly checks.
> I'd like to propose some changes in HLog interface that would ease the multi
> WAL story:
> 1) Have two interfaces WAL and WALService. WAL provides APIs for
> implementors. WALService provides APIs for users (such as RegionServer).
> 2) A skeleton implementation of the above two interface as the base class for
> other WAL implementations (AbstractWAL). It provides required fields for all
> subclasses (fs, conf, log dir, etc). Make a minimal set of test only methods
> and add this set in AbstractWAL.
> 3) HLogFactory returns a WALService reference when creating a WAL instance;
> if a user need to access impl specific APIs (there are unit tests which get
> WAL from a HRegionServer and then call impl specific APIs), use AbstractWAL
> type casting,
> 4) Make TestHLog abstract and let all implementors provide their respective
> test class which extends TestHLog (TestFSHLog, for example).
--
This message was sent by Atlassian JIRA
(v6.1.5#6160)
