-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-25922 - Disabled sanity checks ignored on snapshot restore #4533
Changes from all commits
5678a7b
4070d09
fc1374e
ed59341
da9ba20
09858ce
685b8f6
879ab3e
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 |
---|---|---|
|
@@ -27,6 +27,7 @@ | |
import java.util.concurrent.ScheduledExecutorService; | ||
import java.util.concurrent.ThreadPoolExecutor; | ||
import java.util.concurrent.TimeUnit; | ||
import org.apache.hadoop.conf.Configuration; | ||
import org.apache.hadoop.hbase.CompatibilitySingletonFactory; | ||
import org.apache.hadoop.hbase.HBaseClassTestRule; | ||
import org.apache.hadoop.hbase.HBaseTestingUtil; | ||
|
@@ -40,9 +41,11 @@ | |
import org.apache.hadoop.hbase.testclassification.LargeTests; | ||
import org.apache.hadoop.hbase.testclassification.RegionServerTests; | ||
import org.apache.hadoop.hbase.util.EnvironmentEdgeManagerTestHelper; | ||
import org.apache.hadoop.hbase.util.TableDescriptorChecker; | ||
import org.apache.hadoop.metrics2.MetricsExecutor; | ||
import org.junit.AfterClass; | ||
import org.junit.Assert; | ||
import org.junit.BeforeClass; | ||
import org.junit.ClassRule; | ||
import org.junit.Test; | ||
import org.junit.experimental.categories.Category; | ||
|
@@ -60,6 +63,14 @@ public class TestOpenRegionFailedMemoryLeak { | |
|
||
private static HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil(); | ||
|
||
@BeforeClass | ||
public static void startCluster() throws Exception { | ||
Configuration conf = TEST_UTIL.getConfiguration(); | ||
|
||
// Enable sanity check for coprocessor | ||
conf.setBoolean(TableDescriptorChecker.TABLE_SANITY_CHECKS, true); | ||
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. Why we need this change? 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. Oh, it's due to the fact that this test checks region open is failing due to invalid loaded coprocessor at line 90 below, before this change it was always true (since the coprocessor loading was independent of sanity check). However after this change, we have made it configurable based on the sanity check config (which is always false in hbase-server's test related hbase-site.xml), so we need to explicitly enable sanity check in the test setup 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. OK, seems to have some side effects. Need to fill a release note. |
||
} | ||
|
||
@AfterClass | ||
public static void tearDown() throws IOException { | ||
EnvironmentEdgeManagerTestHelper.reset(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the place where we use logWarn in this method? Then let's move the
boolean logWarn = !shouldSanityCheck(conf);
line here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are multiple checks based on logWarn variable before this line (for e.g. at line no 92, 106, 127, 134 above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, OK, in unchanged code.