Skip to content
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

Jump to bottom

PHOENIX-6627 remove tephra references - WIP #1455

Closed
wants to merge 1 commit into from
Closed

PHOENIX-6627 remove tephra references - WIP #1455

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 0 additions & 3 deletions phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java
View file
Open in desktop
Expand Up @@ -1230,9 +1230,6 @@ public void testAddingRowTimestampColumnNotAllowedViaAlterTable() throws Excepti

@Test
public void testCreatingTxnTableFailsIfTxnsDisabled() throws Exception {
if (!TransactionFactory.Provider.getDefault().runTests()) {
return;
}
Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
props.setProperty(QueryServices.TRANSACTIONS_ENABLED, Boolean.toString(false));
try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
Expand Down
10 changes: 4 additions & 6 deletions phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableWithViewsIT.java
View file
Open in desktop
Expand Up @@ -45,7 +45,6 @@
import org.apache.hadoop.hbase.client.Scan;
import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.phoenix.coprocessor.TephraTransactionalProcessor;
import org.apache.phoenix.exception.SQLExceptionCode;
import org.apache.phoenix.jdbc.PhoenixConnection;
import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
Expand All @@ -67,6 +66,7 @@
import org.apache.phoenix.util.SchemaUtil;
import org.apache.phoenix.util.TestUtil;
import org.junit.Assume;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -987,10 +987,8 @@ public void testDivergedViewsStayDiverged() throws Exception {
}

@Test
@Ignore("tephra dependent test case")
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of @Ignore-ing this test, we could just change it to test for OmidTransactionaProcessor instead.

public void testMakeBaseTableTransactional() throws Exception {
if (!TransactionFactory.Provider.TEPHRA.runTests()) {
return;
}
Properties props = PropertiesUtil.deepCopy(TestUtil.TEST_PROPERTIES);
props.setProperty(QueryServices.TRANSACTIONS_ENABLED, Boolean.TRUE.toString());
try (Connection conn = DriverManager.getConnection(getUrl(), props);
Expand All @@ -1013,7 +1011,7 @@ public void testMakeBaseTableTransactional() throws Exception {
PName tenantId = isMultiTenant ? PNameFactory.newName(TENANT1) : null;
PhoenixConnection phoenixConn = conn.unwrap(PhoenixConnection.class);
Table htable = phoenixConn.getQueryServices().getTable(Bytes.toBytes(baseTableName));
assertFalse(htable.getTableDescriptor().getCoprocessors().contains(TephraTransactionalProcessor.class.getName()));
//assertFalse(htable.getTableDescriptor().getCoprocessors().contains(TephraTransactionalProcessor.class.getName()));
assertFalse(phoenixConn.getTable(new PTableKey(null, baseTableName)).isTransactional());
assertFalse(viewConn.unwrap(PhoenixConnection.class).getTable(new PTableKey(tenantId, viewOfTable)).isTransactional());

Expand All @@ -1023,7 +1021,7 @@ public void testMakeBaseTableTransactional() throws Exception {
// query the view to force the table cache to be updated
viewConn.createStatement().execute("SELECT * FROM " + viewOfTable);
htable = phoenixConn.getQueryServices().getTable(Bytes.toBytes(baseTableName));
assertTrue(htable.getTableDescriptor().getCoprocessors().contains(TephraTransactionalProcessor.class.getName()));
//assertTrue(htable.getTableDescriptor().getCoprocessors().contains(TephraTransactionalProcessor.class.getName()));
assertTrue(phoenixConn.getTable(new PTableKey(null, baseTableName)).isTransactional());
assertTrue(viewConn.unwrap(PhoenixConnection.class).getTable(new PTableKey(tenantId, viewOfTable)).isTransactional());
}
Expand Down
5 changes: 2 additions & 3 deletions phoenix-core/src/it/java/org/apache/phoenix/end2end/IndexToolIT.java
View file
Open in desktop
Expand Up @@ -201,9 +201,8 @@ public static synchronized Collection<Object[]> data() {
boolean[] Booleans = new boolean[] { false, true };
for (boolean namespaceMapped : Booleans) {
for (boolean useSnapshot : Booleans) {
for (String transactionProvider : new String[] {"TEPHRA", "OMID", null}) {
if(transactionProvider !=null &&
!TransactionFactory.Provider.valueOf(transactionProvider).runTests()) {
for (String transactionProvider : new String[] {"OMID", null}) {
if(transactionProvider != null) {
continue;
}
for (boolean mutable : Booleans) {
Expand Down
2 changes: 0 additions & 2 deletions phoenix-core/src/it/java/org/apache/phoenix/end2end/index/ImmutableIndexIT.java
View file
Open in desktop
Expand Up @@ -137,9 +137,7 @@ public static synchronized Collection<Object[]> data() {
Arrays.asList(new Object[][] {
{ false, false, null, false }, { false, false, null, true },
{ false, true, "OMID", false },
{ false, true, "TEPHRA", false }, { false, true, "TEPHRA", true },
{ true, false, null, false }, { true, false, null, true },
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we have a single combination that uses OMID, and four that uses Tephra (probably to save on execution time).
We should still have remove the Omid one, and change the Tephra ones to Omid instead (i.e., instead of five transactional combinations, we should have four)

Copy link
Contributor

Choose a reason for hiding this comment

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

Correction:
Those seem to be unsupported operations for Omid, so this is fine.

{ true, true, "TEPHRA", false }, { true, true, "TEPHRA", true },
}), 2);
}

Expand Down
12 changes: 5 additions & 7 deletions phoenix-core/src/it/java/org/apache/phoenix/rpc/UpdateCacheIT.java
View file
Open in desktop
Expand Up @@ -83,13 +83,11 @@ private static void setupSystemTable(String fullTableName) throws SQLException {
@Test
public void testUpdateCacheForTxnTable() throws Exception {
for (TransactionFactory.Provider provider : TransactionFactory.Provider.values()) {
if (provider.runTests()) {
String tableName = generateUniqueName();
String fullTableName = INDEX_DATA_SCHEMA + QueryConstants.NAME_SEPARATOR + tableName;
Connection conn = DriverManager.getConnection(getUrl(), PropertiesUtil.deepCopy(TEST_PROPERTIES));
conn.createStatement().execute("create table " + fullTableName + TestUtil.TEST_TABLE_SCHEMA + "TRANSACTIONAL=true,TRANSACTION_PROVIDER='" + provider + "'");
helpTestUpdateCache(fullTableName, new int[] {1, 1}, false);
}
String tableName = generateUniqueName();
String fullTableName = INDEX_DATA_SCHEMA + QueryConstants.NAME_SEPARATOR + tableName;
Connection conn = DriverManager.getConnection(getUrl(), PropertiesUtil.deepCopy(TEST_PROPERTIES));
conn.createStatement().execute("create table " + fullTableName + TestUtil.TEST_TABLE_SCHEMA + "TRANSACTIONAL=true,TRANSACTION_PROVIDER='" + provider + "'");
helpTestUpdateCache(fullTableName, new int[] {1, 1}, false);
}
}

Expand Down
5 changes: 2 additions & 3 deletions phoenix-core/src/it/java/org/apache/phoenix/tx/ParameterizedTransactionIT.java
View file
Open in desktop
Expand Up @@ -43,7 +43,6 @@
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.phoenix.coprocessor.TephraTransactionalProcessor;
import org.apache.phoenix.end2end.ParallelStatsDisabledIT;
import org.apache.phoenix.end2end.ParallelStatsDisabledTest;
import org.apache.phoenix.exception.SQLExceptionCode;
Expand Down Expand Up @@ -305,9 +304,9 @@ public void testNonTxToTxTable() throws Exception {
}

htable = conn.unwrap(PhoenixConnection.class).getQueryServices().getTable(Bytes.toBytes( nonTxTableName));
assertTrue(htable.getDescriptor().getCoprocessors().contains(TephraTransactionalProcessor.class.getName()));
//assertTrue(htable.getDescriptor().getCoprocessors().contains(TephraTransactionalProcessor.class.getName()));
Copy link
Contributor

Choose a reason for hiding this comment

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

again, check for the Omid coprocessor instead.

htable = conn.unwrap(PhoenixConnection.class).getQueryServices().getTable(Bytes.toBytes(index));
assertTrue(htable.getDescriptor().getCoprocessors().contains(TephraTransactionalProcessor.class.getName()));
//assertTrue(htable.getDescriptor().getCoprocessors().contains(TephraTransactionalProcessor.class.getName()));

conn.createStatement().execute("UPSERT INTO " + nonTxTableName + " VALUES (4, 'c')");
ResultSet rs = conn.createStatement().executeQuery("SELECT /*+ NO_INDEX */ k FROM " + nonTxTableName + " WHERE v IS NULL");
Expand Down
12 changes: 5 additions & 7 deletions phoenix-core/src/it/java/org/apache/phoenix/tx/TransactionIT.java
View file
Open in desktop
Expand Up @@ -191,12 +191,10 @@ public void testWithMixOfTxProviders() throws Exception {
Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
for (TransactionFactory.Provider provider : TransactionFactory.Provider.values()) {
if (provider.runTests()) {
String tableName = generateUniqueName();
tableNames.add(tableName);
conn.createStatement().execute(
"CREATE TABLE " + tableName + " (k VARCHAR NOT NULL PRIMARY KEY, v1 VARCHAR) TRANSACTIONAL=true,TRANSACTION_PROVIDER='" + provider + "'");
}
String tableName = generateUniqueName();
tableNames.add(tableName);
conn.createStatement().execute(
"CREATE TABLE " + tableName + " (k VARCHAR NOT NULL PRIMARY KEY, v1 VARCHAR) TRANSACTIONAL=true,TRANSACTION_PROVIDER='" + provider + "'");
}
if (tableNames.size() < 2) {
return;
Expand Down Expand Up @@ -231,7 +229,7 @@ public void testPreventLocalIndexCreation() throws Exception {
Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
for (TransactionFactory.Provider provider : TransactionFactory.Provider.values()) {
if (provider.runTests() && provider.getTransactionProvider().isUnsupported(PhoenixTransactionProvider.Feature.ALLOW_LOCAL_INDEX)) {
if (provider.getTransactionProvider().isUnsupported(PhoenixTransactionProvider.Feature.ALLOW_LOCAL_INDEX)) {
String tableName = generateUniqueName();
conn.createStatement().execute(
"CREATE TABLE " + tableName + " (k VARCHAR NOT NULL PRIMARY KEY, v1 VARCHAR) TRANSACTIONAL=true,TRANSACTION_PROVIDER='" + provider + "'");
Expand Down
2 changes: 1 addition & 1 deletion phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
View file
Open in desktop
Expand Up @@ -1255,7 +1255,7 @@ private PTable getTableFromCells(List<Cell> tableCellList, List<List<Cell>> allC
transactionalKv.getValueOffset(),
transactionalKv.getValueLength()))) {
// For backward compat, prior to client setting TRANSACTION_PROVIDER
transactionProvider = TransactionFactory.Provider.TEPHRA;
transactionProvider = TransactionFactory.Provider.OMID;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a behaviour change for old clients that only expect Tephra to exist.
Maybe we should just error out in this case, as the expected functionality has been removed.

I am assuming from the comment the the Omid aware clients always set the Provider, but haven't checked it.

}
} else {
transactionProvider = TransactionFactory.Provider.fromCode(
Expand Down
52 changes: 0 additions & 52 deletions phoenix-core/src/main/java/org/apache/phoenix/coprocessor/TephraTransactionalProcessor.java
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java
View file
Open in desktop
Expand Up @@ -1867,7 +1867,7 @@ public static PTable createFromProto(PTableProtos.PTable table) {
transactionProvider = TransactionFactory.Provider.fromCode(table.getTransactionProvider());
} else if (table.hasTransactional()) {
// For backward compatibility prior to transactionProvider field
transactionProvider = TransactionFactory.Provider.TEPHRA;
transactionProvider = TransactionFactory.Provider.OMID;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should consider erroring out in this case as well.

}
ViewType viewType = null;
String viewStatement = null;
Expand Down
2 changes: 1 addition & 1 deletion ...ix-core/src/main/java/org/apache/phoenix/transaction/NotAvailableTransactionProvider.java
View file
Open in desktop
Expand Up @@ -60,7 +60,7 @@ public PhoenixTransactionClient getTransactionClient(Configuration config, Conne

@Override
public Provider getProvider() {
return TransactionFactory.Provider.TEPHRA;
return TransactionFactory.Provider.OMID;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not right.
We should either return null, or the UnavailableProvider's type, if we keep that,

}

@Override
Expand Down