sijie commented on a change in pull request #1319: Issue 1316: A bookie with
non-writable dirs should be able to start in readonly mode
URL: https://github.com/apache/bookkeeper/pull/1319#discussion_r179810160
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
##########
@@ -398,7 +401,11 @@ synchronized long getLeastUnflushedLogId() {
}
synchronized long getCurrentLogId() {
- return logChannel.getLogId();
+ return currentLogId;
+ }
+
+ BufferedLogChannel getCurrentLogChannel() {
Review comment:
> just like getcurrentlogid is called from SortedLedgerStorage,
> for my entrylogmanager for entrylogperledger I would return null for this
method
as I pointed out at different places, log id stuffs are implementation
specific and they should be hidden to implementation of EntryLogManager. they
don't belong to the interface of EntryLogManager. I proposed in the other
comment to have a method at EntryLogManager `write(SkipLIstFlusher ...)`, to
write a skip list to entry logger, in this way you can hide the
single-entrylog-manager implementation into this method without interfering
with `getCurrentLogId`. so SortedLedgerStorage will not call `getCurrentLogId`
any more but the tests can still examine `getCurrentLogId` through
single-log-manager-implementation at white-box testing.
But again this is a discussion should be in #1281 rather than here.
> currentLogId and one more method - getcurrentlogchannel, when your test
cases are just checking if it is null or not null. It is extra burden to keep
these vaiables in sync currentlogchannel and currentLogId and maintain one more
method
That's a valid point to ask rather than from EntryLogManager perspective. I
will remove `getCurrentLogChannel`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
