Skip to content

PHOENIX-6179 : Relax maxLookBackAge check during an upgrade#942

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.

Sign up for GitHub

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

Closed
virajjasani wants to merge 1 commit into from
Closed

PHOENIX-6179 : Relax maxLookBackAge check during an upgrade #942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 69 additions & 16 deletions phoenix-core/src/it/java/org/apache/phoenix/end2end/BackwardCompatibilityIT.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -56,6 +56,7 @@
import org.apache.hadoop.hbase.client.ConnectionFactory;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.util.VersionInfo;
import org.apache.phoenix.compat.hbase.coprocessor.CompatBaseScannerRegionObserver;
import org.apache.phoenix.coprocessor.MetaDataProtocol;
import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
import org.apache.phoenix.jdbc.PhoenixDriver;
Expand DownExpand Up@@ -115,6 +116,11 @@ public class BackwardCompatibilityIT {
private static String url;
private String tmpDir;

private enum UpgradeProps {
NONE,
SET_MAX_LOOK_BACK_AGE
}

public BackwardCompatibilityIT(String compatibleClientVersion) {
this.compatibleClientVersion = compatibleClientVersion;
}
Expand DownExpand Up@@ -183,15 +189,25 @@ private static List<String> computeClientVersions() throws Exception {
public void testUpsertWithOldClient() throws Exception {
// Insert data with old client and read with new client
executeQueryWithClientVersion(compatibleClientVersion, CREATE_ADD);
executeQueriesWithCurrentVersion(QUERY_CREATE_ADD);
executeQueriesWithCurrentVersion(QUERY_CREATE_ADD, UpgradeProps.NONE);
assertExpectedOutput(QUERY_CREATE_ADD);
}

@Test
public void testCreateDivergedViewWithOldClientReadFromNewClient() throws Exception {
// Create a base table, view and make it diverge from an old client
executeQueryWithClientVersion(compatibleClientVersion, CREATE_DIVERGED_VIEW);
executeQueriesWithCurrentVersion(QUERY_CREATE_DIVERGED_VIEW);
executeQueriesWithCurrentVersion(QUERY_CREATE_DIVERGED_VIEW, UpgradeProps.NONE);
assertExpectedOutput(QUERY_CREATE_DIVERGED_VIEW);
}

@Test
public void testCreateDivergedViewWithOldClientReadWithMaxLookBackAge()
throws Exception {
// Create a base table, view and make it diverge from an old client
executeQueryWithClientVersion(compatibleClientVersion, CREATE_DIVERGED_VIEW);
executeQueriesWithCurrentVersion(QUERY_CREATE_DIVERGED_VIEW,
UpgradeProps.SET_MAX_LOOK_BACK_AGE);
assertExpectedOutput(QUERY_CREATE_DIVERGED_VIEW);
}

Expand All@@ -218,15 +234,15 @@ public void testCreateDivergedViewWithOldClientReadFromOldClientAfterUpgrade()

@Test
public void testCreateDivergedViewWithNewClientReadFromOldClient() throws Exception {
executeQueriesWithCurrentVersion(CREATE_DIVERGED_VIEW);
executeQueriesWithCurrentVersion(CREATE_DIVERGED_VIEW, UpgradeProps.NONE);
executeQueryWithClientVersion(compatibleClientVersion, QUERY_CREATE_DIVERGED_VIEW);
assertExpectedOutput(QUERY_CREATE_DIVERGED_VIEW);
}

@Test
public void testCreateDivergedViewWithNewClientReadFromNewClient() throws Exception {
executeQueriesWithCurrentVersion(CREATE_DIVERGED_VIEW);
executeQueriesWithCurrentVersion(QUERY_CREATE_DIVERGED_VIEW);
executeQueriesWithCurrentVersion(CREATE_DIVERGED_VIEW, UpgradeProps.NONE);
executeQueriesWithCurrentVersion(QUERY_CREATE_DIVERGED_VIEW, UpgradeProps.NONE);
assertExpectedOutput(QUERY_CREATE_DIVERGED_VIEW);
}

Expand All@@ -241,7 +257,7 @@ public void testCreateDivergedViewWithNewClientReadFromNewClient() throws Except
@Test
public void testSelectWithOldClient() throws Exception {
// Insert data with new client and read with old client
executeQueriesWithCurrentVersion(CREATE_ADD);
executeQueriesWithCurrentVersion(CREATE_ADD, UpgradeProps.NONE);
executeQueryWithClientVersion(compatibleClientVersion, QUERY_CREATE_ADD);
assertExpectedOutput(QUERY_CREATE_ADD);
}
Expand All@@ -253,18 +269,43 @@ public void testSelectWithOldClient() throws Exception {
* 3. New Client reads the data inserted by the old client
* 4. New Client inserts more data into the tables created by old client
* 5. Old Client reads the data inserted by new client
* Use phoenix.max.lookback.age.seconds config and ensure that upgrade
* is not impacted by the config.
*
* @throws Exception thrown if any errors encountered during query execution or file IO
*/
@Test
public void testSelectUpsertWithNewClientWithMaxLookBackAge() throws Exception {
// Insert data with old client and read with new client
executeQueryWithClientVersion(compatibleClientVersion, CREATE_ADD);
executeQueriesWithCurrentVersion(QUERY_CREATE_ADD, UpgradeProps.SET_MAX_LOOK_BACK_AGE);
assertExpectedOutput(QUERY_CREATE_ADD);

// Insert more data with new client and read with old client
executeQueriesWithCurrentVersion(ADD_DATA, UpgradeProps.SET_MAX_LOOK_BACK_AGE);
executeQueryWithClientVersion(compatibleClientVersion, QUERY_ADD_DATA);
assertExpectedOutput(QUERY_ADD_DATA);
}

/**
* Scenario:
* 1. Old Client connects to the updated server
* 2. Old Client creates tables and inserts data
* 3. New Client reads the data inserted by the old client
* 4. New Client inserts more data into the tables created by old client
* 5. Old Client reads the data inserted by new client
*
* @throws Exception thrown if any errors encountered during query execution or file IO
*/
@Test
public void testSelectUpsertWithNewClient() throws Exception {
// Insert data with old client and read with new client
executeQueryWithClientVersion(compatibleClientVersion, CREATE_ADD);
executeQueriesWithCurrentVersion(QUERY_CREATE_ADD);
executeQueriesWithCurrentVersion(QUERY_CREATE_ADD, UpgradeProps.NONE);
assertExpectedOutput(QUERY_CREATE_ADD);

// Insert more data with new client and read with old client
executeQueriesWithCurrentVersion(ADD_DATA);
executeQueriesWithCurrentVersion(ADD_DATA, UpgradeProps.NONE);
executeQueryWithClientVersion(compatibleClientVersion, QUERY_ADD_DATA);
assertExpectedOutput(QUERY_ADD_DATA);
}
Expand All@@ -282,13 +323,13 @@ public void testSelectUpsertWithNewClient() throws Exception {
@Test
public void testSelectUpsertWithOldClient() throws Exception {
// Insert data with new client and read with old client
executeQueriesWithCurrentVersion(CREATE_ADD);
executeQueriesWithCurrentVersion(CREATE_ADD, UpgradeProps.NONE);
executeQueryWithClientVersion(compatibleClientVersion, QUERY_CREATE_ADD);
assertExpectedOutput(QUERY_CREATE_ADD);

// Insert more data with old client and read with new client
executeQueryWithClientVersion(compatibleClientVersion, ADD_DATA);
executeQueriesWithCurrentVersion(QUERY_ADD_DATA);
executeQueriesWithCurrentVersion(QUERY_ADD_DATA, UpgradeProps.NONE);
assertExpectedOutput(QUERY_ADD_DATA);
}

Expand All@@ -305,7 +346,7 @@ public void testSelectUpsertWithOldClient() throws Exception {
public void testUpsertDeleteWithOldClient() throws Exception {
// Insert data with old client and read with new client
executeQueryWithClientVersion(compatibleClientVersion, CREATE_ADD);
executeQueriesWithCurrentVersion(QUERY_CREATE_ADD);
executeQueriesWithCurrentVersion(QUERY_CREATE_ADD, UpgradeProps.NONE);
assertExpectedOutput(QUERY_CREATE_ADD);

// Deletes with the old client
Expand All@@ -326,21 +367,22 @@ public void testUpsertDeleteWithOldClient() throws Exception {
@Test
public void testUpsertDeleteWithNewClient() throws Exception {
// Insert data with old client and read with new client
executeQueriesWithCurrentVersion(CREATE_ADD);
executeQueriesWithCurrentVersion(CREATE_ADD, UpgradeProps.NONE);
executeQueryWithClientVersion(compatibleClientVersion, QUERY_CREATE_ADD);
assertExpectedOutput(QUERY_CREATE_ADD);

// Deletes with the new client
executeQueriesWithCurrentVersion(ADD_DELETE);
executeQueriesWithCurrentVersion(QUERY_ADD_DELETE);
executeQueriesWithCurrentVersion(ADD_DELETE, UpgradeProps.NONE);
executeQueriesWithCurrentVersion(QUERY_ADD_DELETE, UpgradeProps.NONE);
assertExpectedOutput(QUERY_ADD_DELETE);
}

@Test
public void testUpdatedSplitPolicyForSysTask() throws Exception {
executeQueryWithClientVersion(compatibleClientVersion,
CREATE_DIVERGED_VIEW);
executeQueriesWithCurrentVersion(QUERY_CREATE_DIVERGED_VIEW);
executeQueriesWithCurrentVersion(QUERY_CREATE_DIVERGED_VIEW,
UpgradeProps.NONE);
try (org.apache.hadoop.hbase.client.Connection conn =
hbaseTestUtil.getConnection(); Admin admin = conn.getAdmin()) {
TableDescriptor tableDescriptor = admin.getDescriptor(
Expand DownExpand Up@@ -437,8 +479,19 @@ public void run() {
}

// Executes the SQL commands listed in the given operation file from the sql_files directory
private void executeQueriesWithCurrentVersion(String operation) throws Exception {
private void executeQueriesWithCurrentVersion(String operation,
UpgradeProps upgradeProps) throws Exception {
Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
if (UpgradeProps.SET_MAX_LOOK_BACK_AGE.equals(upgradeProps)) {
// any value < 31 is enough to test relaxing the MaxLookBack age
// checks during an upgrade because during upgrade, SCN for the
// connection is set to be the phoenix version timestamp
// (31 as of now: MIN_SYSTEM_TABLE_TIMESTAMP_4_16_0 / MIN_SYSTEM_TABLE_TIMESTAMP_5_1_0)
// Hence, keeping value: 15
props.put(CompatBaseScannerRegionObserver.PHOENIX_MAX_LOOKBACK_AGE_CONF_KEY,
Integer.toString(15));

@yanxinyiyanxinyiOct 30, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why 15 here?

@virajjasanivirajjasaniOct 30, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With 15, we are able to reproduce the exception without source code change on this patch. Is it fine? I can update this if there is any better suggestion that I might be unaware of.

@virajjasanivirajjasaniOct 30, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any value less than 31 should work for testing purpose. Because during upgrade, SCN is set to be Phoenix version timestamp:

 public static final long MIN_SYSTEM_TABLE_TIMESTAMP_4_16_0 = MIN_TABLE_TIMESTAMP + 31;
public static final long MIN_SYSTEM_TABLE_TIMESTAMP_5_1_0 = MIN_SYSTEM_TABLE_TIMESTAMP_4_16_0;

@yanxinyiyanxinyiOct 30, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add explanation comments here so that in the future ppl have a better understanding, thanks

@virajjasanivirajjasaniOct 30, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done @yanxinyi
Thanks

}

try (Connection conn = DriverManager.getConnection(url, props)) {
StringBuilder sb = new StringBuilder();
try (BufferedReader reader =
Expand Down
5 changes: 5 additions & 0 deletions phoenix-core/src/main/java/org/apache/phoenix/compile/QueryCompiler.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -183,6 +183,11 @@ private void verifySCN() throws SQLException {
return;
}
PhoenixConnection conn = statement.getConnection();
if (conn.isRunningUpgrade()) {
// PHOENIX-6179 : if upgrade is going on, we don't need to
// perform MaxLookBackAge check
return;
}
Long scn = conn.getSCN();
if (scn == null) {
return;
Expand Down