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

PHOENIX-7484 Optimize mutation plan creation for upserts over tenant connection #2041

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+92 −9
Open

PHOENIX-7484 Optimize mutation plan creation for upserts over tenant connection #2041

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3535736
PHOENIX-7484 Optimize mutation plan creation for upserts over tenant …
Dec 13, 2024
4ac8c81
Address Viraj's comment
Dec 13, 2024
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
21 changes: 15 additions & 6 deletions phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -761,13 +761,22 @@ public PTable getTable(String name) throws SQLException {
}

public PTable getTable(PTableKey key) throws SQLException {
PTable table;
try {
table = getTableRef(key).getTable();
} catch (TableNotFoundException e) {
table = getTableNoCache(key.getName());
PTableRef tableRef = getTableRefOptimized(key);
if (tableRef == null && key.getTenantId() != null) {
tableRef = getTableRefOptimized(new PTableKey(null, key.getName()));
}
return table;
if (tableRef == null) {
return getTableNoCache(key.getName());
}
return tableRef.getTable();
}

public PTableRef getTableRefOptimized(PTableKey key) {
PTableRef tableRef = getQueryServices().getMetaDataCache().getTableRefOptimized(key);
if (tableRef != null && prune(tableRef.getTable())) {
return null;
}
return tableRef;
}

public PTableRef getTableRef(PTableKey key) throws TableNotFoundException {
Expand Down
1 change: 1 addition & 0 deletions phoenix-core-client/src/main/java/org/apache/phoenix/schema/PMetaData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public static interface Pruner {
}
public int size();
public PTableRef getTableRef(PTableKey key) throws TableNotFoundException;
public PTableRef getTableRefOptimized(PTableKey key);
public void pruneTables(Pruner pruner);
public PFunction getFunction(PTableKey key) throws FunctionNotFoundException;
public void pruneFunctions(Pruner pruner);
Expand Down
15 changes: 14 additions & 1 deletion phoenix-core-client/src/main/java/org/apache/phoenix/schema/PMetaDataImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,27 @@ private void updateGlobalMetric(PTableRef pTableRef) {

@Override
public PTableRef getTableRef(PTableKey key) throws TableNotFoundException {
return getTableRef(key, false);
}

@Override
public PTableRef getTableRefOptimized(PTableKey key) {
try {
return getTableRef(key, true);
} catch (TableNotFoundException e) {
throw new IllegalStateException(e);
}
}

private PTableRef getTableRef(PTableKey key, boolean doNotThrowTableNotFoundException) throws TableNotFoundException {
Copy link
Contributor

Choose a reason for hiding this comment

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

With this, you also mean to cover PHOENIX-7486?

Copy link
Contributor Author

@sanjeet006py sanjeet006py Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, PHOENIX-7486 is a more of generic JIRA for finding and fixing other occurrences of this anti pattern. I fixed one in here as that was contributing to the regressed numbers. W/o the change in PMetadataImpl I was able to bring down regression to 30-40% from 5K-6K% but need this change to bring it down to 0.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: use skipTNFE instead of doNotThrowTableNotFoundException

if (physicalNameToLogicalTableMap.containsKey(key.getName())) {
key = physicalNameToLogicalTableMap.get(key.getName());
}
PTableRef ref = metaData.get(key);
if (!key.getName().contains(QueryConstants.SYSTEM_SCHEMA_NAME)) {
updateGlobalMetric(ref);
}
if (ref == null) {
if (ref == null && ! doNotThrowTableNotFoundException) {
throw new TableNotFoundException(key.getName());
}
return ref;
Expand Down
11 changes: 11 additions & 0 deletions phoenix-core-client/src/main/java/org/apache/phoenix/schema/PSynchronizedMetaData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,17 @@ public PTableRef getTableRef(PTableKey key) throws TableNotFoundException {
}
}

@Override
public PTableRef getTableRefOptimized(PTableKey key) {
readWriteLock.readLock().lock();
try {
return delegate.getTableRefOptimized(key);
}
finally {
readWriteLock.readLock().unlock();
}
}

@Override
public void updateResolvedTimestamp(PTable table, long resolvedTimestamp) throws SQLException {
readWriteLock.writeLock().lock();
Expand Down
53 changes: 51 additions & 2 deletions phoenix-core/src/test/java/org/apache/phoenix/schema/PMetaDataImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.apache.phoenix.query.QueryServices;
import org.apache.phoenix.util.ReadOnlyProps;
import org.apache.phoenix.util.TimeKeeper;
import org.junit.Assert;
import org.junit.Test;

import org.apache.phoenix.thirdparty.com.google.common.collect.Maps;
Expand All @@ -39,13 +40,23 @@
public class PMetaDataImplTest {

private static void addToTable(PMetaData metaData, String name, int size, TestTimeKeeper timeKeeper) throws SQLException {
PTable table = new PSizedTable(new PTableKey(null,name), size);
addToTable(metaData, null, name, size, timeKeeper);
}

private static void addToTable(PMetaData metaData, String tenantId, String name, int size, TestTimeKeeper timeKeeper) throws SQLException {
PName keyTenantId = tenantId == null ? null : new PNameImpl(tenantId);
PTable table = new PSizedTable(new PTableKey(keyTenantId, name), size);
metaData.addTable(table, System.currentTimeMillis());
timeKeeper.incrementTime();
}

private static void removeFromTable(PMetaData metaData, String name, TestTimeKeeper timeKeeper) throws SQLException {
metaData.removeTable(null, name, null, HConstants.LATEST_TIMESTAMP);
removeFromTable(metaData, null, name, timeKeeper);
}

private static void removeFromTable(PMetaData metaData, String tenantId, String name, TestTimeKeeper timeKeeper) throws SQLException {
PName keyTenantId = tenantId == null ? null : new PNameImpl(tenantId);
metaData.removeTable(keyTenantId, name, null, HConstants.LATEST_TIMESTAMP);
timeKeeper.incrementTime();
}

Expand All @@ -54,6 +65,13 @@ private static PTable getFromTable(PMetaData metaData, String name, TestTimeKeep
timeKeeper.incrementTime();
return table;
}

private static PTableRef getTableRefOptimized(PMetaData metaData, String tenantId, String name, TestTimeKeeper timeKeeper) {
PName keyTenantId = tenantId == null ? null : new PNameImpl(tenantId);
PTableRef tableRef = metaData.getTableRefOptimized(new PTableKey(keyTenantId, name));
timeKeeper.incrementTime();
return tableRef;
}

private static void assertNames(PMetaData metaData, String... names) {
Set<String> actualTables = Sets.newHashSet();
Expand Down Expand Up @@ -198,6 +216,37 @@ public void testSchema() throws Exception {
}
}

@Test
public void testGetTableRefOptimized() throws Exception {
TestTimeKeeper timeKeeper = new TestTimeKeeper();
Map<String, String> props = Maps.newHashMapWithExpectedSize(2);
props.put(QueryServices.MAX_CLIENT_METADATA_CACHE_SIZE_ATTRIB, "10");
props.put(QueryServices.CLIENT_CACHE_ENCODING, "object");
PMetaData metaData = new PMetaDataImpl(5, Long.MAX_VALUE, timeKeeper, new ReadOnlyProps(props));
String tenantId = "t1";
String tableName = "a";

// Test for PTableRef with null tenant Id
addToTable(metaData, null, tableName, 5, timeKeeper);
Assert.assertEquals(1, metaData.size());
PTableRef tableRef = getTableRefOptimized(metaData, null, tableName, timeKeeper);
Assert.assertNotNull(tableRef);
removeFromTable(metaData, null, tableName, timeKeeper);
Assert.assertEquals(0, metaData.size());
tableRef = getTableRefOptimized(metaData, null, tableName, timeKeeper);
Assert.assertNull(tableRef);

// Test for PTableRef with non-null tenant Id
addToTable(metaData, tenantId, tableName, 5, timeKeeper);
Assert.assertEquals(1, metaData.size());
tableRef = getTableRefOptimized(metaData, tenantId, tableName, timeKeeper);
Assert.assertNotNull(tableRef);
removeFromTable(metaData, tenantId, tableName, timeKeeper);
Assert.assertEquals(0, metaData.size());
tableRef = getTableRefOptimized(metaData, tenantId, tableName, timeKeeper);
Assert.assertNull(tableRef);
}

private static class PSizedTable extends PTableImpl {
private final int size;
private final PTableKey key;
Expand Down