saintstack commented on a change in pull request #753: HBASE-23181 Blocked WAL
archive: "LogRoller: Failed to schedule flush…
URL: https://github.com/apache/hbase/pull/753#discussion_r339284636
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WAL.java
##########
@@ -97,44 +97,59 @@
void close() throws IOException;
/**
- * Append a set of edits to the WAL. The WAL is not flushed/sync'd after
this transaction
- * completes BUT on return this edit must have its region edit/sequence id
assigned
- * else it messes up our unification of mvcc and sequenceid. On return
<code>key</code> will
- * have the region edit/sequence id filled in.
+ * Append a set of data edits to the WAL. 'Data' here means that the content
in the edits will
+ * also be added to memstore.
+ * <p/>
+ * The WAL is not flushed/sync'd after this transaction completes BUT on
return this edit must
+ * have its region edit/sequence id assigned else it messes up our
unification of mvcc and
+ * sequenceid. On return <code>key</code> will have the region edit/sequence
id filled in.
* @param info the regioninfo associated with append
* @param key Modified by this call; we add to it this edits region
edit/sequence id.
* @param edits Edits to append. MAY CONTAIN NO EDITS for case where we want
to get an edit
- * sequence id that is after all currently appended edits.
- * @param inMemstore Always true except for case where we are writing a
compaction completion
- * record into the WAL; in this case the entry is just so we can finish an
unfinished compaction
- * -- it is not an edit for memstore.
+ * sequence id that is after all currently appended edits.
* @return Returns a 'transaction id' and <code>key</code> will have the
region edit/sequence id
- * in it.
+ * in it.
+ * @see #appendMarker(RegionInfo, WALKeyImpl, WALEdit, boolean)
*/
- long append(RegionInfo info, WALKeyImpl key, WALEdit edits, boolean
inMemstore) throws IOException;
+ long appendData(RegionInfo info, WALKeyImpl key, WALEdit edits) throws
IOException;
+
+ /**
+ * Append a marker edit to the WAL. A marker could be a FlushDescriptor, a
compaction marker, or
+ * region event marker. The difference here is that, a marker will not be
added to memstore.
+ * <p/>
+ * The WAL is not flushed/sync'd after this transaction completes BUT on
return this edit must
+ * have its region edit/sequence id assigned else it messes up our
unification of mvcc and
+ * sequenceid. On return <code>key</code> will have the region edit/sequence
id filled in.
+ * @param info the regioninfo associated with append
+ * @param key Modified by this call; we add to it this edits region
edit/sequence id.
+ * @param edits Edits to append. MAY CONTAIN NO EDITS for case where we want
to get an edit
+ * sequence id that is after all currently appended edits.
+ * @param closeRegion Whether this is a region close marker, i.e, the last
wal edit for this
Review comment:
I messed w/ the patch. I see how the appendData and appendMarker don't take
us far enough down... down to AbstractFSWAL#appendEntry where we could ask if a
close marker. But looking at patch, what would be wrong w/ doing something like
this?
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java
index 2eb7c7436a..bc31204500 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java
@@ -1001,7 +1001,7 @@ public abstract class AbstractFSWAL<W extends
WriterBase> implements WAL {
doAppend(writer, entry);
assert highestUnsyncedTxid < entry.getTxid();
highestUnsyncedTxid = entry.getTxid();
- if (entry.isCloseRegion()) {
+ if (!entry.isInMemStore() && entry.isCloseMarker()) {
// let's clean all the records of this region
sequenceIdAccounting.onRegionClose(encodedRegionName);
} else {
... where entry.isCloseMarker would do something like WALEdit.isMetaEdit...
public boolean isMetaEdit() {
for (Cell cell: cells) {
if (!isMetaEditFamily(cell)) {
return false;
}
}
return true;
}
... only instead we'd look for METAFAMILY:HBASE::CLOSE, a new define added
on WALEdit?
Thanks for taking a look.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
