-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-26106 AbstractFSWALProvider#getArchivedLogPath doesn't look for wal file in all oldWALs directory. #3636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
9017251
f053161
09ce4db
2965d21
d56db9d
d3f17b6
7e828ae
6e45431
14e5d48
7475397
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,9 @@ | |
|
|
||
| import static org.junit.Assert.assertEquals; | ||
| import static org.junit.Assert.assertFalse; | ||
| import static org.junit.Assert.assertNotEquals; | ||
| import static org.junit.Assert.assertTrue; | ||
| import java.io.IOException; | ||
| import java.util.List; | ||
| import java.util.NavigableMap; | ||
| import java.util.TreeMap; | ||
|
|
@@ -31,6 +33,7 @@ | |
| import org.apache.hadoop.hbase.HBaseTestingUtil; | ||
| import org.apache.hadoop.hbase.HConstants; | ||
| import org.apache.hadoop.hbase.KeyValue; | ||
| import org.apache.hadoop.hbase.ServerName; | ||
| import org.apache.hadoop.hbase.TableName; | ||
| import org.apache.hadoop.hbase.client.RegionInfo; | ||
| import org.apache.hadoop.hbase.client.RegionInfoBuilder; | ||
|
|
@@ -92,6 +95,11 @@ private static String getName() { | |
| return "TestWALRecordReader"; | ||
| } | ||
|
|
||
| private static String getServerName() { | ||
| ServerName serverName = ServerName.valueOf("TestWALRecordReader", 1, 1); | ||
| return serverName.toString(); | ||
| } | ||
|
|
||
| @Before | ||
| public void setUp() throws Exception { | ||
| fs.delete(hbaseDir, true); | ||
|
|
@@ -282,7 +290,6 @@ public void testWALRecordReaderActiveArchiveTolerance() throws Exception { | |
| LOG.debug("log="+logDir+" file="+ split.getLogFileName()); | ||
|
|
||
| testSplitWithMovingWAL(splits.get(0), Bytes.toBytes("1"), Bytes.toBytes("2")); | ||
|
|
||
| } | ||
|
|
||
| protected WALKeyImpl getWalKeyImpl(final long time, NavigableMap<byte[], Integer> scopes) { | ||
|
|
@@ -335,13 +342,16 @@ private void testSplitWithMovingWAL(InputSplit split, byte[] col1, byte[] col2) | |
| // Move log file to archive directory | ||
| // While WAL record reader is open | ||
| WALInputFormat.WALSplit split_ = (WALInputFormat.WALSplit) split; | ||
|
|
||
| Path logFile = new Path(split_.getLogFileName()); | ||
| Path archivedLog = AbstractFSWALProvider.getArchivedLogPath(logFile, conf); | ||
| boolean result = fs.rename(logFile, archivedLog); | ||
| assertTrue(result); | ||
| result = fs.exists(archivedLog); | ||
| assertTrue(result); | ||
| Path archivedLogDir = getWALArchiveDir(conf); | ||
| Path archivedLogLocation = new Path(archivedLogDir, logFile.getName()); | ||
| assertNotEquals(split_.getLogFileName(), archivedLogLocation.toString()); | ||
|
|
||
| assertTrue(fs.rename(logFile, archivedLogLocation)); | ||
| assertTrue(fs.exists(archivedLogDir)); | ||
| assertFalse(fs.exists(logFile)); | ||
| // TODO: This is not behaving as expected. WALInputFormat#WALKeyRecordReader doesn't open | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test is not testing what it is intending to. After this patch, we are able to successfully rename the file to oldWALs directory but somehow it is not triggering the condition within nextKeyValue method to look into archiveDir.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So you mean we should file another issue to address this? Anyway, I think this PR is a critical one as it could cause data loss, so let me merge it to all branches first in case of not blocking the upcoming 2.4.x release.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was going to cut 2.4.6RC0 today but saw this issue. Will wait until this is merged to branch-2.4.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, we need to open a new jira to modify the test. Will file it soon.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| // TODO: the archivedLogLocation to read next key value. | ||
| assertTrue(reader.nextKeyValue()); | ||
| cell = reader.getCurrentValue().getCells().get(0); | ||
| if (!Bytes.equals(col2, 0, col2.length, cell.getQualifierArray(), cell.getQualifierOffset(), | ||
|
|
@@ -353,4 +363,10 @@ private void testSplitWithMovingWAL(InputSplit split, byte[] col1, byte[] col2) | |
| } | ||
| reader.close(); | ||
| } | ||
|
|
||
| private Path getWALArchiveDir(Configuration conf) throws IOException { | ||
| Path rootDir = CommonFSUtils.getWALRootDir(conf); | ||
| String archiveDir = AbstractFSWALProvider.getWALArchiveDirectoryName(conf, getServerName()); | ||
| return new Path(rootDir, archiveDir); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.