From 0f30fab65b79231cfdb03d3dab948aa13b074ff6 Mon Sep 17 00:00:00 2001 From: Rushabh Shah Date: Sun, 5 Nov 2023 17:43:51 -0800 Subject: [PATCH 1/2] HBASE-28184 Tailing the WAL is very slow if there are multiple peers --- .../regionserver/WALEntryStream.java | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/WALEntryStream.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/WALEntryStream.java index cbaf3c6f6e96..5a29abf7aa40 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/WALEntryStream.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/WALEntryStream.java @@ -258,7 +258,26 @@ private void dequeueCurrentLog() throws IOException { private boolean readNextEntryAndRecordReaderPosition() throws IOException { Entry readEntry = reader.next(); long readerPos = reader.getPosition(); - OptionalLong fileLength = walFileLengthProvider.getLogFileSizeIfBeingWritten(currentPath); + OptionalLong fileLength; + if (logQueue.getQueueSize(walGroupId) > 1) { + // if there are more than one files in queue, although it is possible that we are + // still trying to write the trailer of the file and it is not closed yet, we can + // make sure that we will not write any WAL entries to it any more, so it is safe + // to just let the upper layer try to read the whole file without limit + fileLength = OptionalLong.empty(); + } else { + // if there is only one file in queue, check whether it is still being written to + // we must call this before actually reading from the reader, as this method will acquire the + // rollWriteLock. This is very important, as we will enqueue the new WAL file in postLogRoll, + // and before this happens, we could have already finished closing the previous WAL file. If + // we do not acquire the rollWriteLock and return whether the current file is being written + // to, we may finish reading the previous WAL file and start to read the next one, before it + // is enqueued into the logQueue, thus lead to an empty logQueue and make the shipper think + // the queue is already ended and quit.I the future, if we want to optimize the logic here, + // for example, do not call this method every time, or do not acquire rollWriteLock in the + // implementation of this method, we need to carefully review the optimized implementation. + fileLength = walFileLengthProvider.getLogFileSizeIfBeingWritten(currentPath); + } if (fileLength.isPresent() && readerPos > fileLength.getAsLong()) { // See HBASE-14004, for AsyncFSWAL which uses fan-out, it is possible that we read uncommitted // data, so we need to make sure that we do not read beyond the committed file length. From 78a74bed02448a71d54eca82b8db4d15c4645412 Mon Sep 17 00:00:00 2001 From: Rushabh Shah Date: Mon, 6 Nov 2023 12:00:26 -0800 Subject: [PATCH 2/2] Addressing review comments --- .../replication/regionserver/WALEntryStream.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/WALEntryStream.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/WALEntryStream.java index 5a29abf7aa40..0a778cbb1b80 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/WALEntryStream.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/WALEntryStream.java @@ -260,22 +260,9 @@ private boolean readNextEntryAndRecordReaderPosition() throws IOException { long readerPos = reader.getPosition(); OptionalLong fileLength; if (logQueue.getQueueSize(walGroupId) > 1) { - // if there are more than one files in queue, although it is possible that we are - // still trying to write the trailer of the file and it is not closed yet, we can - // make sure that we will not write any WAL entries to it any more, so it is safe - // to just let the upper layer try to read the whole file without limit fileLength = OptionalLong.empty(); } else { // if there is only one file in queue, check whether it is still being written to - // we must call this before actually reading from the reader, as this method will acquire the - // rollWriteLock. This is very important, as we will enqueue the new WAL file in postLogRoll, - // and before this happens, we could have already finished closing the previous WAL file. If - // we do not acquire the rollWriteLock and return whether the current file is being written - // to, we may finish reading the previous WAL file and start to read the next one, before it - // is enqueued into the logQueue, thus lead to an empty logQueue and make the shipper think - // the queue is already ended and quit.I the future, if we want to optimize the logic here, - // for example, do not call this method every time, or do not acquire rollWriteLock in the - // implementation of this method, we need to carefully review the optimized implementation. fileLength = walFileLengthProvider.getLogFileSizeIfBeingWritten(currentPath); } if (fileLength.isPresent() && readerPos > fileLength.getAsLong()) {