[
https://issues.apache.org/jira/browse/PHOENIX-7490?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17906720#comment-17906720
]
Sanjeet Malhotra commented on PHOENIX-7490:
-------------------------------------------
The current MetadataClient#updateCache() call looks into client metadata cache
for tenant view if we are using tenant connection and if no entry is found,
then it looks for a global table. This logic fails a corner case scenario where
we have a tenant view and a global table with the same name and the entry for
tenant view somehow gets ejected, then a tenant connection can wrongly end up
selecting global table.
We can fix this by hitting SYSCAT always when we don't find tenant view in
client metadata cache but this will cause us to always hit SYSCAT if there is
no tenant view in SYSCAT. Thus, to avoid looking up SYSCAT always in the case
of negative lookup we can maintain a negative cache at CQSI level. Thus, once
we hit SYSCAT to get tenant view and find that no such tenant view exists then
we can cache this info to avoid looking up again in the SYSCAT on next call.
The same fix is needed for MetadataClient#updateCache() and PHOENIX-7484. We
also discussed following points to reach to final solution: * Having negative
entries in same CQSI cache as the positive entries. But this can penalize the
non-multi tenant tables as they most likely won't be as frequently accessed as
tenant's base table. So, better to keep these two caches separate.
* Having tenant connection specific negative cache. As we earlier had tenant
connection specific positive cache and deprecated it so, this time good to
start with single CQSI level tenant negative cache. Additionally, if the
negative cache is not shared b/w connections then each connection will be
penalized at least once. Moreover, having a tenant connection negative cache
will require us to maintain caches at two levels i.e. CQSI and
PhoenixConnection, which we would like to avoid.
This bug will be fixed automatically once PHOENIX-7490 gets fixed.
Thanks [~haridsv] [~kadir] for suggesting the solution to handle the bigger
problem.
Working on design doc, will share it here soon.
> Upserts and queries to tenant view end up upserting and querying global table
> if both have same name
> ----------------------------------------------------------------------------------------------------
>
> Key: PHOENIX-7490
> URL: https://issues.apache.org/jira/browse/PHOENIX-7490
> Project: Phoenix
> Issue Type: Bug
> Affects Versions: 5.3.0
> Reporter: Sanjeet Malhotra
> Assignee: Sanjeet Malhotra
> Priority: Major
>
> The following IT can be used to observing the bug:
>
> {code:java}
> @Test
> public void testTenantViewAndGlobalTableWithSameName() throws Exception {
> String tableName = generateUniqueName();
> // Create a global table
> try(Connection conn = DriverManager.getConnection(getUrl())) {
> Statement stmt = conn.createStatement();
> stmt.execute("CREATE TABLE " + tableName + " (org_id varchar not
> null, " +
> "pk1 varchar not null, " +
> "pk2 varchar not null, " +
> "col1 integer constraint PK1 PRIMARY KEY (org_id, pk1, pk2))
> MULTI_TENANT=true");
> }
> Properties props = new Properties();
> props.setProperty(TENANT_ID_ATTRIB, "tenant1");
> String baseTableName = generateUniqueName();
> try(Connection conn = DriverManager.getConnection(getUrl())) {
> Statement stmt = conn.createStatement();
> stmt.execute("CREATE TABLE " + baseTableName + " (org_id varchar not
> null, " +
> "pk3 varchar not null, " +
> "pk4 varchar not null, " +
> "col2 integer constraint PK1 PRIMARY KEY (org_id, pk3, pk4))
> MULTI_TENANT=true");
> }
> // Create a tenant view with same name as global table
> try(Connection conn = DriverManager.getConnection(getUrl(), props)) {
> Statement stmt = conn.createStatement();
> stmt.execute("CREATE VIEW " + tableName + " AS SELECT * FROM " +
> baseTableName);
> PhoenixConnection pconn = conn.unwrap(PhoenixConnection.class);
> PMetaData metadata = pconn.getQueryServices().getMetaDataCache();
> Assert.assertNotNull(pconn.getTableRef(new
> PTableKey(pconn.getTenantId(), tableName)));
> int metadataCacheSize = metadata.size();
> metadata.removeTable(pconn.getTenantId(), tableName, null,
> HConstants.LATEST_TIMESTAMP);
> Assert.assertEquals(metadataCacheSize - 1, metadata.size());
> // Assert successful eviction of tenant view PTable from metadata
> cache
> Assert.assertThrows(TableNotFoundException.class, () ->
> pconn.getTableRef(new PTableKey(pconn.getTenantId(), tableName)));
> // Assert PTable for global table is in the metadata cache
> Assert.assertNotNull(pconn.getTableRef(new PTableKey(null,
> tableName)));
> // Insert a row with intention to insert it in tenant view
> stmt.executeUpdate("UPSERT INTO " + tableName + " VALUES ('val1',
> 'val2', 1)");
> // Assert that PTable for tenant view is still not there in metadata
> cache
> Assert.assertThrows(TableNotFoundException.class, () ->
> pconn.getTableRef(new PTableKey(pconn.getTenantId(), tableName)));
> conn.commit();
> // Assert that PTable for tenant view is still not there in metadata
> cache
> Assert.assertThrows(TableNotFoundException.class, () ->
> pconn.getTableRef(new PTableKey(pconn.getTenantId(), tableName)));
> }
> // Query tenant view with expectation to get 1 row
> try(Connection conn = DriverManager.getConnection(getUrl(), props)) {
> Statement stmt = conn.createStatement();
> ResultSet rs = stmt.executeQuery("SELECT * FROM " + tableName);
> int rowCount = 0;
> while (rs.next()) {
> //TODO: Once the bug is fixed then this assertion should be
> changed to tenant view's PK3
> Assert.assertEquals("PK1", rs.getMetaData().getColumnName(1));
> rowCount++;
> }
> Assert.assertEquals(1, rowCount);
> }
> // Query global table
> try(Connection conn = DriverManager.getConnection(getUrl())) {
> Statement stmt = conn.createStatement();
> ResultSet rs = stmt.executeQuery("SELECT * FROM " + tableName);
> int rowCount = 0;
> while (rs.next()) {
> rowCount++;
> Assert.assertEquals("PK1", rs.getMetaData().getColumnName(2));
> }
> //TODO: Once bug is fixed the expected row count should be changed to > 0
> Assert.assertEquals(1, rowCount);
> stmt = conn.createStatement();
> rs = stmt.executeQuery("SELECT COUNT(*) FROM " + baseTableName);
> while (rs.next()) {
> //TODO: Once bug is fixed the expected row count should be
> changed to 1
> Assert.assertEquals(0, rs.getInt(1));
> }
> }
> } {code}
> Steps to reproduce:
> # Add above IT to UpsertValuesIT and it will succeed.
> # If we address all the TODOs in the IT then it will fail. After addressing
> all the TODOs we get the expected behaviour once this bug is fixed.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
