Uh oh!
There was an error while loading. Please reload this page.
HBASE-28184 Addendum PR - #5521
Open
shahrs87 wants to merge 2 commits into
Open
Conversation
shahrs87
commented
Nov 14, 2023
| long readerPos = reader.getPosition(); | ||
| OptionalLong fileLength; | ||
| if (logQueue.getQueueSize(walGroupId) > 1) { | ||
| if (logQueue.getQueueSize(walGroupId) > 2) { |
ContributorAuthor
There was a problem hiding this comment.
Checking for less than equal to 2 log files in the queue because it is possible that we are doing this check just when the WAL is rolled and the reader has not read the trailer bytes of the old WAL.
ContributorAuthor
There was a problem hiding this comment.
Contributor
There was a problem hiding this comment.
Changing to greater than 2 can fix the failing tests? A bit strange, could you please exlain more on this?
ContributorAuthor
There was a problem hiding this comment.
Any updates here?
@Apache9 Sorry couldn't update this thread in a long time. Got distracted somewhere and it fell off my radar.
Changing to greater than 2 can fix the failing tests? A bit strange, could you please exlain more on this?
Actually changing to greater than 2 fixes the failing test but looks like it is not the right fix.
The test is doing the following:
- Creating WAL named wal1
- Appending some entries to wal1
- Calling entryStream.next to read from wal1
- Roll the WAL to wal2
- Append some entries to wal2
- Call entryStream.next to read from wal2
- Test that there are NO uncleanlyClosedLogs metric.
The test is failing at #6 above. When it is calling entryStream.next on wal2, the replication code needs to switch the reader to the new WAL file. During rollWriter, we add it to AbstractFSWAL#inflightWALClosures map and close the old WAL file asynchronously here.
In closeWriter method, we append the trailer to the WAL and then close it.
During the closeWriter execution there will be 2 WALs in the logQueue.
Now in WALEntryStream#next method, after this change, we don't read the file length if logQueue size is greater than 1 and hence WALEntryStream is unaware of the trailer bytes and while switching the wal from wal1 to wal2, it gets the following exception:
2024-01-17T10:05:47,247 DEBUG [Listener at localhost/52964] wal.ProtobufLogReader(447): Encountered a malformed edit, seeking back to last good position in file, from 218 to 210
java.io.EOFException: Invalid PB, EOF? Ignoring; originalPosition=210, currentPosition=218
at org.apache.hadoop.hbase.regionserver.wal.ProtobufLogReader.readNext(ProtobufLogReader.java:376) ~[classes/:?]
at org.apache.hadoop.hbase.regionserver.wal.ReaderBase.next(ReaderBase.java:104) ~[classes/:?]
at org.apache.hadoop.hbase.regionserver.wal.ReaderBase.next(ReaderBase.java:92) ~[classes/:?]
at org.apache.hadoop.hbase.replication.regionserver.WALEntryStream.readNextEntryAndRecordReaderPosition(WALEntryStream.java:259) ~[classes/:?]
at org.apache.hadoop.hbase.replication.regionserver.WALEntryStream.tryAdvanceEntry(WALEntryStream.java:181) ~[classes/:?]
at org.apache.hadoop.hbase.replication.regionserver.WALEntryStream.hasNext(WALEntryStream.java:102) ~[classes/:?]
at org.apache.hadoop.hbase.replication.regionserver.WALEntryStream.peek(WALEntryStream.java:111) ~[classes/:?]
at org.apache.hadoop.hbase.replication.regionserver.WALEntryStream.next(WALEntryStream.java:118) ~[classes/:?]
at org.apache.hadoop.hbase.replication.regionserver.WALEntryStreamTestBase$WALEntryStreamWithRetries.access$001(WALEntryStreamTestBase.java:82) ~[test-classes/:?]
at org.apache.hadoop.hbase.replication.regionserver.WALEntryStreamTestBase$WALEntryStreamWithRetries.lambda$next$0(WALEntryStreamTestBase.java:95) ~[test-classes/:?]
at org.apache.hadoop.hbase.Waiter.waitFor(Waiter.java:184) ~[test-classes/:?]
at org.apache.hadoop.hbase.Waiter.waitFor(Waiter.java:135) ~[test-classes/:?]
at org.apache.hadoop.hbase.replication.regionserver.WALEntryStreamTestBase$WALEntryStreamWithRetries.next(WALEntryStreamTestBase.java:94) ~[test-classes/:?]
at org.apache.hadoop.hbase.replication.regionserver.TestBasicWALEntryStream.testCleanClosedWALs(TestBasicWALEntryStream.java:726) ~[test-classes/:?]
I think I know how to fix.
From PR-5505, we have the below check
OptionalLong fileLength;
if (logQueue.getQueueSize(walGroupId) > 1) {
fileLength = OptionalLong.empty();
} else {
// if there is only one file in queue, check whether it is still being written to
fileLength = walFileLengthProvider.getLogFileSizeIfBeingWritten(currentPath);
}
Along with checking queue size, we also have to check if the currently replicated WAL is not in AbstractFSWAL#inflightWALClosures map then it is safe to not read the file size.
But currently there is NO way to access AbstractFSWAL object from WALEntryStream. I found one class named WALFileLengthProvider but that is a functional interface and as the name suggests it only provides log file size if that file is currently being written.
@Apache9 Any ideas on how can we expose AbstractFSWAL object to WALEntryStream?
ContributorAuthor
There was a problem hiding this comment.
@Apache9 Can you please take a look in my prev comment?
Apache-HBase
commented
Nov 14, 2023
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Nov 14, 2023
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Nov 14, 2023
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Nov 14, 2023
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Nov 15, 2023
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Nov 15, 2023
🎊 +1 overall
This message was automatically generated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.