[
https://issues.apache.org/jira/browse/PHOENIX-6627?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17577142#comment-17577142
]
ASF GitHub Bot commented on PHOENIX-6627:
-----------------------------------------
stoty commented on code in PR #1455:
URL: https://github.com/apache/phoenix/pull/1455#discussion_r940893331
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableWithViewsIT.java:
##########
@@ -987,10 +987,8 @@ public void testDivergedViewsStayDiverged() throws
Exception {
}
@Test
+ @Ignore("tephra dependent test case")
Review Comment:
Instead of @Ignore-ing this test, we could just change it to test for
OmidTransactionaProcessor instead.
##########
phoenix-core/src/it/java/org/apache/phoenix/tx/ParameterizedTransactionIT.java:
##########
@@ -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()));
Review Comment:
again, check for the Omid coprocessor instead.
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java:
##########
@@ -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;
Review Comment:
We should consider erroring out in this case as well.
##########
phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java:
##########
@@ -1268,27 +1268,8 @@ public static void assertResultSet(ResultSet rs,
Object[][] rows) throws Excepti
}
public static Collection<Object[]> filterTxParamData(Collection<Object[]>
data, int index) {
- boolean runAllTests = true;
- boolean runNoTests = true;
-
- for (TransactionFactory.Provider provider :
TransactionFactory.Provider.values()) {
- runAllTests &= provider.runTests();
- runNoTests &= !provider.runTests();
- }
- if (runNoTests) {
- return Collections.emptySet();
- }
- if (runAllTests) {
- return data;
- }
- List<Object[]> filteredData =
Lists.newArrayListWithExpectedSize(data.size());
- for (Object[] params : data) {
- String provider = (String) params[index];
- if (provider == null ||
TransactionFactory.Provider.valueOf(provider).runTests()) {
- filteredData.add(params);
- }
- }
- return filteredData;
+ return data;
+ //TODO: remove this method altogether
Review Comment:
Yes, it shouldn't be too difficult.
##########
phoenix-core/src/main/java/org/apache/phoenix/transaction/TransactionFactory.java:
##########
@@ -94,7 +66,7 @@ public static PhoenixTransactionProvider
getTransactionProvider(byte[] txState,
return null;
}
Provider provider = (clientVersion <
MetaDataProtocol.MIN_SYSTEM_TABLE_TIMESTAMP_4_14_0)
- ? Provider.TEPHRA
+ ? Provider.OMID
Review Comment:
Again, should we just error out ?
##########
phoenix-core/src/main/java/org/apache/phoenix/transaction/NotAvailableTransactionProvider.java:
##########
@@ -60,7 +60,7 @@ public PhoenixTransactionClient
getTransactionClient(Configuration config, Conne
@Override
public Provider getProvider() {
- return TransactionFactory.Provider.TEPHRA;
+ return TransactionFactory.Provider.OMID;
Review Comment:
This is not right.
We should either return null, or the UnavailableProvider's type, if we keep
that,
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/ImmutableIndexIT.java:
##########
@@ -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 },
Review Comment:
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)
##########
phoenix-core/src/main/java/org/apache/phoenix/transaction/TransactionFactory.java:
##########
@@ -22,43 +22,19 @@
public class TransactionFactory {
- private static final PhoenixTransactionProvider tephraTransactionProvider;
-
- static{
- boolean tephraEnabled = true;
- try {
- Class.forName("org.apache.tephra.TransactionFailureException");
- } catch (Throwable e) {
- tephraEnabled = false;
- }
- if (tephraEnabled) {
- tephraTransactionProvider =
TephraTransactionProvider.getInstance();
- } else {
- tephraTransactionProvider =
NotAvailableTransactionProvider.getInstance();
- }
- }
-
public enum Provider {
- TEPHRA((byte)1, tephraTransactionProvider,
- tephraTransactionProvider instanceof TephraTransactionProvider),
- OMID((byte)2, OmidTransactionProvider.getInstance(), true);
+ OMID((byte)2, OmidTransactionProvider.getInstance());
Review Comment:
This changes the ordinal, which SHOULD not be a real problem as we don't use
that for serialization, but keeping TEPHRA, and just renaming it to something
like TEPHRA_REMOVED would possibly also make it easier to handle the hairy
cases where tephra defined for old tables.
##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -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;
Review Comment:
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.
> Remove all references to Tephra from 4.x and master
> ---------------------------------------------------
>
> Key: PHOENIX-6627
> URL: https://issues.apache.org/jira/browse/PHOENIX-6627
> Project: Phoenix
> Issue Type: Sub-task
> Components: 4.x, tephra
> Reporter: Istvan Toth
> Assignee: Zsolt Gyulavari
> Priority: Major
> Fix For: 5.2.0
>
>
> Removing tephra from the runtime is easy, as it uses the well defind
> TransactionProvider interfaces.
> Removing Tephra references from all the test cases is a much bigger task.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
