PHOENIX-4997 Phoenix MR on snapshots can produce duplicate rows #397
Conversation
|
|
||
| @BeforeClass | ||
| public static void doSetup() throws Exception { | ||
| Map<String,String> props = Maps.newHashMapWithExpectedSize(1); | ||
| setUpTestDriver(new ReadOnlyProps(props.entrySet().iterator())); | ||
| } | ||
|
|
||
| @Test | ||
| public void testMapReduceSnapshots() throws Exception { | ||
| @Before |
There was a problem hiding this comment.
Refactoring done to move common code to before method.
| conn.commit(); | ||
| } | ||
|
|
||
| public void deleteSnapshot(String tableName) throws Exception { | ||
| private void splitTableSync(Admin admin, TableName hbaseTableName, |
There was a problem hiding this comment.
Didn't use the method provided in BaseTest.java since enabling/disabling the table causes the snapshot manifest file to not contain the offline regions.
The bug is caused because the snapshot manifest file on a regular production table can contain information about regions that are split or offline, which should ideally be ignored when restoring snapshot.
|
@BinShi-SecularBird |
| } | ||
|
|
||
| // Exclude offline split parent regions | ||
| private boolean isValidRegion(RegionInfo hri) { |
There was a problem hiding this comment.
Maybe extract this to a util since its used in two classes.
There was a problem hiding this comment.
I thought about it, however they don't have any classes in common and the implementations of these methods are slightly different. I agree that this is duplicate code and should not exist this way but I couldn't think of a better way for this.
| if (inputQuery == null) | ||
| inputQuery = selectQuery.toString(); | ||
|
|
||
| ResultSet rs = DriverManager.getConnection(getUrl(), props).createStatement().executeQuery(inputQuery); | ||
|
|
||
| for (List<Object> r : result) { | ||
| assertTrue("No data stored in the table!", rs.next()); | ||
| if (rs.next() == false) { | ||
| System.out.println("Placeholder"); |
|
lgtm. |
|
Thanks @twdsilva and @gjacoby126 for the review. Will push it soon. |
@akshita-malhotra @gjacoby126
Could you please review?