Github user samarthjain commented on a diff in the pull request: https://github.com/apache/phoenix/pull/129#discussion_r45289517 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/index/MutableIndexFailureIT.java --- @@ -135,230 +154,282 @@ public void tearDown() throws Exception { @Ignore("See PHOENIX-2331") @Test(timeout=300000) public void testWriteFailureDisablesLocalIndex() throws Exception { - testWriteFailureDisablesIndex(true); + helpTestWriteFailureDisablesIndex(true); } @Ignore("See PHOENIX-2332") @Test(timeout=300000) public void testWriteFailureDisablesIndex() throws Exception { - testWriteFailureDisablesIndex(false); + helpTestWriteFailureDisablesIndex(false); } - public void testWriteFailureDisablesIndex(boolean localIndex) throws Exception { - String query; - ResultSet rs; - + public void helpTestWriteFailureDisablesIndex(boolean localIndex) throws Exception { Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES); - Connection conn = driver.connect(url, props); - conn.setAutoCommit(false); - conn.createStatement().execute( - "CREATE TABLE " + DATA_TABLE_FULL_NAME + " (k VARCHAR NOT NULL PRIMARY KEY, v1 VARCHAR, v2 VARCHAR)"); - query = "SELECT * FROM " + DATA_TABLE_FULL_NAME; - rs = conn.createStatement().executeQuery(query); - assertFalse(rs.next()); - - if(localIndex) { - conn.createStatement().execute( - "CREATE LOCAL INDEX " + INDEX_TABLE_NAME + " ON " + DATA_TABLE_FULL_NAME + " (v1) INCLUDE (v2)"); - conn.createStatement().execute( - "CREATE LOCAL INDEX " + INDEX_TABLE_NAME+ "_2" + " ON " + DATA_TABLE_FULL_NAME + " (v2) INCLUDE (v1)"); - } else { + try (Connection conn = driver.connect(url, props)) { + String query; + ResultSet rs; + conn.setAutoCommit(false); conn.createStatement().execute( - "CREATE INDEX " + INDEX_TABLE_NAME + " ON " + DATA_TABLE_FULL_NAME + " (v1) INCLUDE (v2)"); - } + "CREATE TABLE " + DATA_TABLE_FULL_NAME + " (k VARCHAR NOT NULL PRIMARY KEY, v1 VARCHAR, v2 VARCHAR)"); + query = "SELECT * FROM " + DATA_TABLE_FULL_NAME; + rs = conn.createStatement().executeQuery(query); + assertFalse(rs.next()); + + if(localIndex) { + conn.createStatement().execute( + "CREATE LOCAL INDEX " + INDEX_TABLE_NAME + " ON " + DATA_TABLE_FULL_NAME + " (v1) INCLUDE (v2)"); + conn.createStatement().execute( + "CREATE LOCAL INDEX " + INDEX_TABLE_NAME+ "_2" + " ON " + DATA_TABLE_FULL_NAME + " (v2) INCLUDE (v1)"); + } else { + conn.createStatement().execute( + "CREATE INDEX " + INDEX_TABLE_NAME + " ON " + DATA_TABLE_FULL_NAME + " (v1) INCLUDE (v2)"); + } + + query = "SELECT * FROM " + INDEX_TABLE_FULL_NAME; + rs = conn.createStatement().executeQuery(query); + assertFalse(rs.next()); + + // Verify the metadata for index is correct. + rs = conn.getMetaData().getTables(null, StringUtil.escapeLike(SCHEMA_NAME), INDEX_TABLE_NAME, + new String[] { PTableType.INDEX.toString() }); + assertTrue(rs.next()); + assertEquals(INDEX_TABLE_NAME, rs.getString(3)); + assertEquals(PIndexState.ACTIVE.toString(), rs.getString("INDEX_STATE")); + assertFalse(rs.next()); - query = "SELECT * FROM " + INDEX_TABLE_FULL_NAME; - rs = conn.createStatement().executeQuery(query); - assertFalse(rs.next()); - - // Verify the metadata for index is correct. - rs = conn.getMetaData().getTables(null, StringUtil.escapeLike(SCHEMA_NAME), INDEX_TABLE_NAME, - new String[] { PTableType.INDEX.toString() }); - assertTrue(rs.next()); - assertEquals(INDEX_TABLE_NAME, rs.getString(3)); - assertEquals(PIndexState.ACTIVE.toString(), rs.getString("INDEX_STATE")); - assertFalse(rs.next()); - - PreparedStatement stmt = conn.prepareStatement("UPSERT INTO " + DATA_TABLE_FULL_NAME + " VALUES(?,?,?)"); - stmt.setString(1, "a"); - stmt.setString(2, "x"); - stmt.setString(3, "1"); - stmt.execute(); - conn.commit(); - - TableName indexTable = - TableName.valueOf(localIndex ? MetaDataUtil - .getLocalIndexTableName(DATA_TABLE_FULL_NAME) : INDEX_TABLE_FULL_NAME); - HBaseAdmin admin = this.util.getHBaseAdmin(); - HTableDescriptor indexTableDesc = admin.getTableDescriptor(indexTable); - try{ - admin.disableTable(indexTable); - admin.deleteTable(indexTable); - } catch (TableNotFoundException ignore) {} - - stmt = conn.prepareStatement("UPSERT INTO " + DATA_TABLE_FULL_NAME + " VALUES(?,?,?)"); - stmt.setString(1, "a2"); - stmt.setString(2, "x2"); - stmt.setString(3, "2"); - stmt.execute(); - try { + PreparedStatement stmt = conn.prepareStatement("UPSERT INTO " + DATA_TABLE_FULL_NAME + " VALUES(?,?,?)"); + stmt.setString(1, "a"); + stmt.setString(2, "x"); + stmt.setString(3, "1"); + stmt.execute(); conn.commit(); - } catch (SQLException e) {} - - // Verify the metadata for index is correct. - rs = conn.getMetaData().getTables(null, StringUtil.escapeLike(SCHEMA_NAME), INDEX_TABLE_NAME, - new String[] { PTableType.INDEX.toString() }); - assertTrue(rs.next()); - assertEquals(INDEX_TABLE_NAME, rs.getString(3)); - assertEquals(PIndexState.DISABLE.toString(), rs.getString("INDEX_STATE")); - assertFalse(rs.next()); - if(localIndex) { - rs = conn.getMetaData().getTables(null, StringUtil.escapeLike(SCHEMA_NAME), INDEX_TABLE_NAME+"_2", - new String[] { PTableType.INDEX.toString() }); + + TableName indexTable = + TableName.valueOf(localIndex ? MetaDataUtil + .getLocalIndexTableName(DATA_TABLE_FULL_NAME) : INDEX_TABLE_FULL_NAME); + HBaseAdmin admin = this.util.getHBaseAdmin(); + HTableDescriptor indexTableDesc = admin.getTableDescriptor(indexTable); + try{ + admin.disableTable(indexTable); + admin.deleteTable(indexTable); + } catch (TableNotFoundException ignore) {} + + stmt = conn.prepareStatement("UPSERT INTO " + DATA_TABLE_FULL_NAME + " VALUES(?,?,?)"); + stmt.setString(1, "a2"); + stmt.setString(2, "x2"); + stmt.setString(3, "2"); + stmt.execute(); + if (transactional) { + try { + conn.commit(); + fail(); + } catch (SQLException e1) { + try { + conn.rollback(); + fail(); + } catch (SQLException e2) { + // rollback fails as well because index is disabled + } + } + } + else { + conn.commit(); + } + + // Verify the metadata for index is correct. + rs = conn.getMetaData().getTables(null, StringUtil.escapeLike(SCHEMA_NAME), INDEX_TABLE_NAME, + new String[] { PTableType.INDEX.toString() }); assertTrue(rs.next()); - assertEquals(INDEX_TABLE_NAME+"_2", rs.getString(3)); + assertEquals(INDEX_TABLE_NAME, rs.getString(3)); assertEquals(PIndexState.DISABLE.toString(), rs.getString("INDEX_STATE")); assertFalse(rs.next()); - } - - // Verify UPSERT on data table still work after index is disabled - stmt = conn.prepareStatement("UPSERT INTO " + DATA_TABLE_FULL_NAME + " VALUES(?,?,?)"); - stmt.setString(1, "a3"); - stmt.setString(2, "x3"); - stmt.setString(3, "3"); - stmt.execute(); - conn.commit(); - - query = "SELECT v2 FROM " + DATA_TABLE_FULL_NAME + " where v1='x3'"; - rs = conn.createStatement().executeQuery("EXPLAIN " + query); - assertTrue(QueryUtil.getExplainPlan(rs).contains("CLIENT PARALLEL 1-WAY FULL SCAN OVER " + DATA_TABLE_FULL_NAME)); - rs = conn.createStatement().executeQuery(query); - assertTrue(rs.next()); - - // recreate index table - admin.createTable(indexTableDesc); - do { - Thread.sleep(15 * 1000); // sleep 15 secs - rs = conn.getMetaData().getTables(null, StringUtil.escapeLike(SCHEMA_NAME), INDEX_TABLE_NAME, - new String[] { PTableType.INDEX.toString() }); - assertTrue(rs.next()); - if(PIndexState.ACTIVE.toString().equals(rs.getString("INDEX_STATE"))){ - break; - } - if(localIndex) { - rs = conn.getMetaData().getTables(null, StringUtil.escapeLike(SCHEMA_NAME), INDEX_TABLE_NAME+"_2", + if(localIndex) { + rs = conn.getMetaData().getTables(null, StringUtil.escapeLike(SCHEMA_NAME), INDEX_TABLE_NAME+"_2", + new String[] { PTableType.INDEX.toString() }); + assertTrue(rs.next()); + assertEquals(INDEX_TABLE_NAME+"_2", rs.getString(3)); --- End diff -- Very minor nit: space before and after +
--- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---