dlg99 commented on code in PR #3493:
URL: https://github.com/apache/bookkeeper/pull/3493#discussion_r975790016
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java:
##########
@@ -310,16 +379,21 @@ public void onSizeLimitReached(final Checkpoint cp)
throws IOException {
scheduler.execute(new Runnable() {
@Override
public void run() {
- try {
- LOG.info("Started flushing mem table.");
-
interleavedLedgerStorage.getEntryLogger().prepareEntryMemTableFlush();
- memTable.flush(SortedLedgerStorage.this);
- if
(interleavedLedgerStorage.getEntryLogger().commitEntryMemTableFlush()) {
-
interleavedLedgerStorage.checkpointer.startCheckpoint(cp);
+ for (InterleavedLedgerStorage s :
interleavedLedgerStorageList) {
+ try {
+ LOG.info("ledgerStorage({}) started flushing mem
table.",
+ getEntryLogDirPath(s.getEntryLogger()));
+ s.getEntryLogger().prepareEntryMemTableFlush();
+ memTable.flush(SortedLedgerStorage.this);
+
+ if (s.getEntryLogger().commitEntryMemTableFlush()) {
Review Comment:
it is possible that only some interleaved storages / entry loggers actually
got data (e.g. memetaable had data for one ledger only). What's the side-effect
of the prepare/commit flush without any actual data? Do we have tests for this?
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java:
##########
@@ -310,16 +379,21 @@ public void onSizeLimitReached(final Checkpoint cp)
throws IOException {
scheduler.execute(new Runnable() {
@Override
public void run() {
- try {
- LOG.info("Started flushing mem table.");
-
interleavedLedgerStorage.getEntryLogger().prepareEntryMemTableFlush();
- memTable.flush(SortedLedgerStorage.this);
- if
(interleavedLedgerStorage.getEntryLogger().commitEntryMemTableFlush()) {
-
interleavedLedgerStorage.checkpointer.startCheckpoint(cp);
+ for (InterleavedLedgerStorage s :
interleavedLedgerStorageList) {
+ try {
+ LOG.info("ledgerStorage({}) started flushing mem
table.",
+ getEntryLogDirPath(s.getEntryLogger()));
+ s.getEntryLogger().prepareEntryMemTableFlush();
+ memTable.flush(SortedLedgerStorage.this);
Review Comment:
I think there is no need to do memetable.flush for each interleaved storage,
Sorted storage should fan flush out.
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java:
##########
@@ -310,16 +379,21 @@ public void onSizeLimitReached(final Checkpoint cp)
throws IOException {
scheduler.execute(new Runnable() {
@Override
public void run() {
- try {
- LOG.info("Started flushing mem table.");
-
interleavedLedgerStorage.getEntryLogger().prepareEntryMemTableFlush();
- memTable.flush(SortedLedgerStorage.this);
- if
(interleavedLedgerStorage.getEntryLogger().commitEntryMemTableFlush()) {
-
interleavedLedgerStorage.checkpointer.startCheckpoint(cp);
+ for (InterleavedLedgerStorage s :
interleavedLedgerStorageList) {
+ try {
+ LOG.info("ledgerStorage({}) started flushing mem
table.",
+ getEntryLogDirPath(s.getEntryLogger()));
+ s.getEntryLogger().prepareEntryMemTableFlush();
+ memTable.flush(SortedLedgerStorage.this);
+
+ if (s.getEntryLogger().commitEntryMemTableFlush()) {
+ s.checkpointer.startCheckpoint(cp);
+ }
+ } catch (Exception e) {
+ s.getStateManager().transitionToReadOnlyMode();
Review Comment:
Now we create interleaved storages (ILS) per directory
(`ledgerDirsManager.getAllLedgerDirs().size()`)
Entry log manager (EntryLogManagerBase,
getCurrentLogForLedgerForAddEntry/createNewLog/selectDirForNextEntryLog) still
rotates entry logs across all available directories (the only reason why
transition to read only on one ILS failing write is acceptable.) Also there is
entry log per ledger.
It looks like it implicitly introduces parallelism (N ILSs for N dirs ->
will write to N entry loggers, possibly at the same dir) which might not be ok
for rotational disks
i do not have clear picture of what specifically you are trying to achieve
so this per my best understanding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
