You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ra...@apache.org on 2020/05/11 14:14:43 UTC
[phoenix] branch 4.x updated: PHOENIX-4753 Remove the need for
users to have Write access to the Phoenix SYSTEM STATS TABLE to drop
tables(Rajeshbabu)
This is an automated email from the ASF dual-hosted git repository.
rajeshbabu pushed a commit to branch 4.x
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/4.x by this push:
new 2152a41 PHOENIX-4753 Remove the need for users to have Write access to the Phoenix SYSTEM STATS TABLE to drop tables(Rajeshbabu)
2152a41 is described below
commit 2152a41bcefc5dbefd4355aa967d85f125c639d8
Author: Rajeshbabu Chintaguntla <rc...@cloudera.com>
AuthorDate: Mon May 11 19:40:43 2020 +0530
PHOENIX-4753 Remove the need for users to have Write access to the Phoenix SYSTEM STATS TABLE to drop tables(Rajeshbabu)
---
.../apache/phoenix/end2end/BasePermissionsIT.java | 127 +++++++++++++++++++--
.../phoenix/coprocessor/MetaDataEndpointImpl.java | 58 +++++++++-
.../coprocessor/PhoenixAccessController.java | 10 +-
.../org/apache/phoenix/schema/MetaDataClient.java | 27 -----
.../java/org/apache/phoenix/util/MetaDataUtil.java | 61 ++++++----
5 files changed, 223 insertions(+), 60 deletions(-)
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
index 218c6b1..ffa724f 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BasePermissionsIT.java
@@ -38,6 +38,7 @@ import org.apache.phoenix.query.QueryConstants;
import org.apache.phoenix.query.QueryServices;
import org.apache.phoenix.schema.NewerSchemaAlreadyExistsException;
import org.apache.phoenix.schema.TableNotFoundException;
+import org.apache.phoenix.util.MetaDataUtil;
import org.apache.phoenix.util.PhoenixRuntime;
import org.apache.phoenix.util.SchemaUtil;
import org.junit.Before;
@@ -64,11 +65,7 @@ import java.util.List;
import java.util.Properties;
import java.util.Set;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
@Category(NeedsOwnMiniClusterTest.class)
@FixMethodOrder(MethodSorters.NAME_ASCENDING)
@@ -207,6 +204,12 @@ public abstract class BasePermissionsIT extends BaseTest {
conf.set(QueryServices.IS_NAMESPACE_MAPPING_ENABLED, Boolean.toString(isNamespaceMapped));
}
+ private static void configureStatsConfigurations(Configuration conf) {
+ conf.set(QueryServices.STATS_GUIDEPOST_WIDTH_BYTES_ATTRIB, Long.toString(20));
+ conf.set(QueryServices.STATS_UPDATE_FREQ_MS_ATTRIB, Long.toString(5));
+ conf.set(QueryServices.MAX_SERVER_METADATA_CACHE_TIME_TO_LIVE_MS_ATTRIB, Long.toString(5));
+ conf.set(QueryServices.USE_STATS_FOR_PARALLELIZATION, Boolean.toString(true));
+ }
public static HBaseTestingUtility getUtility(){
return testUtil;
}
@@ -383,13 +386,17 @@ public abstract class BasePermissionsIT extends BaseTest {
}
AccessTestAction createTable(final String tableName) throws SQLException {
+ return createTable(tableName, NUM_RECORDS);
+ }
+
+ AccessTestAction createTable(final String tableName, final int numRecordsToInsert) throws SQLException {
return new AccessTestAction() {
@Override
public Object run() throws Exception {
try (Connection conn = getConnection(); Statement stmt = conn.createStatement();) {
assertFalse(stmt.execute("CREATE TABLE " + tableName + "(pk INTEGER not null primary key, data VARCHAR, val integer)"));
try (PreparedStatement pstmt = conn.prepareStatement("UPSERT INTO " + tableName + " values(?, ?, ?)")) {
- for (int i = 0; i < NUM_RECORDS; i++) {
+ for (int i = 0; i < numRecordsToInsert; i++) {
pstmt.setInt(1, i);
pstmt.setString(2, Integer.toString(i));
pstmt.setInt(3, i);
@@ -403,6 +410,19 @@ public abstract class BasePermissionsIT extends BaseTest {
};
}
+ AccessTestAction updateStatsOnTable(final String tableName) throws SQLException {
+ return new AccessTestAction() {
+ @Override
+ public Object run() throws Exception {
+ try (Connection conn = getConnection(); Statement stmt = conn.createStatement();) {
+ assertFalse(stmt.execute("UPDATE STATISTICS " + tableName + " SET \""
+ + QueryServices.STATS_GUIDEPOST_WIDTH_BYTES_ATTRIB + "\" = 5"));
+ }
+ return null;
+ }
+ };
+ }
+
private AccessTestAction createMultiTenantTable(final String tableName) throws SQLException {
return new AccessTestAction() {
@Override
@@ -439,6 +459,37 @@ public abstract class BasePermissionsIT extends BaseTest {
}
+ private AccessTestAction deleteDataFromStatsTable() throws SQLException {
+ return new AccessTestAction() {
+ @Override
+ public Object run() throws Exception {
+ try (Connection conn = getConnection(); Statement stmt = conn.createStatement();) {
+ conn.setAutoCommit(true);
+ assertNotEquals(0, stmt.executeUpdate("DELETE FROM SYSTEM.STATS"));
+ }
+ return null;
+ }
+ };
+
+ }
+
+ private AccessTestAction readStatsAfterTableDelete(final String physicalTableName) throws SQLException {
+ return new AccessTestAction() {
+ @Override
+ public Object run() throws Exception {
+ try (Connection conn = getConnection(); Statement stmt = conn.createStatement();) {
+ conn.setAutoCommit(true);
+ ResultSet rs = stmt.executeQuery("SELECT count(*) from SYSTEM.STATS" +
+ " WHERE PHYSICAL_NAME = '"+ physicalTableName +"'");
+ rs.next();
+ assertEquals(0, rs.getInt(1));
+ }
+ return null;
+ }
+ };
+
+ }
+
// Attempts to read given table without verifying data
// AccessDeniedException is only triggered when ResultSet#next() method is called
// The first call triggers HBase Scan object
@@ -1219,6 +1270,69 @@ public abstract class BasePermissionsIT extends BaseTest {
}
@Test
+ public void testDeletingStatsShouldNotFailWithADEWhenTableDropped() throws Throwable {
+ final String schema = "STATS_ENABLED";
+ final String tableName = "DELETE_TABLE_IT";
+ final String phoenixTableName = schema + "." + tableName;
+ final String indexName1 = tableName + "_IDX1";
+ final String lIndexName1 = tableName + "_LIDX1";
+ final String viewName1 = schema+"."+tableName + "_V1";
+ final String viewIndexName1 = tableName + "_VIDX1";
+ grantSystemTableAccess();
+ try {
+ superUser1.runAs(new PrivilegedExceptionAction<Void>() {
+ @Override
+ public Void run() throws Exception {
+ try {
+ verifyAllowed(createSchema(schema), superUser1);
+ //Neded Global ADMIN for flush operation during drop table
+ AccessControlClient.grant(getUtility().getConnection(),regularUser1.getName(), Permission.Action.ADMIN);
+ if (isNamespaceMapped) {
+ grantPermissions(regularUser1.getName(), schema, Permission.Action.CREATE);
+ grantPermissions(AuthUtil.toGroupEntry(GROUP_SYSTEM_ACCESS), schema, Permission.Action.CREATE);
+ } else {
+ grantPermissions(regularUser1.getName(),
+ NamespaceDescriptor.DEFAULT_NAMESPACE.getName(), Permission.Action.CREATE);
+ grantPermissions(AuthUtil.toGroupEntry(GROUP_SYSTEM_ACCESS),
+ NamespaceDescriptor.DEFAULT_NAMESPACE.getName(), Permission.Action.CREATE);
+ }
+ } catch (Throwable e) {
+ if (e instanceof Exception) {
+ throw (Exception)e;
+ } else {
+ throw new Exception(e);
+ }
+ }
+ return null;
+ }
+ });
+
+ verifyAllowed(createTable(phoenixTableName, 100), regularUser1);
+ verifyAllowed(createIndex(indexName1,phoenixTableName),regularUser1);
+ verifyAllowed(createLocalIndex(lIndexName1, phoenixTableName), regularUser1);
+ verifyAllowed(createView(viewName1,phoenixTableName),regularUser1);
+ verifyAllowed(createIndex(viewIndexName1, viewName1), regularUser1);
+ verifyAllowed(updateStatsOnTable(phoenixTableName), regularUser1);
+ Thread.sleep(10000);
+ // Normal deletes should fail when no write permissions given on stats table.
+ verifyDenied(deleteDataFromStatsTable(), AccessDeniedException.class, regularUser1);
+ verifyAllowed(dropIndex(viewIndexName1, viewName1), regularUser1);
+ verifyAllowed(dropView(viewName1),regularUser1);
+ verifyAllowed(dropIndex(indexName1, phoenixTableName), regularUser1);
+ Thread.sleep(3000);
+ verifyAllowed(readStatsAfterTableDelete(SchemaUtil.getPhysicalHBaseTableName(
+ schema, indexName1, isNamespaceMapped).getString()), regularUser1);
+ verifyAllowed(dropIndex(lIndexName1, phoenixTableName), regularUser1);
+ verifyAllowed(dropTable(phoenixTableName), regularUser1);
+ Thread.sleep(3000);
+ verifyAllowed(readStatsAfterTableDelete(SchemaUtil.getPhysicalHBaseTableName(
+ schema, tableName, isNamespaceMapped).getString()), regularUser1);
+ } finally {
+ revokeAll();
+ }
+ }
+
+ @Test
public void testUpsertIntoImmutableTable() throws Throwable {
final String schema = generateUniqueName();
final String tableName = generateUniqueName();
@@ -1291,5 +1405,4 @@ public abstract class BasePermissionsIT extends BaseTest {
}
};
}
-
}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
index 02fd037..7c9dcc5 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
@@ -2195,7 +2195,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso
String tableType = request.getTableType();
byte[] schemaName = null;
byte[] tableName = null;
-
+ boolean dropTableStats = false;
try {
List<Mutation> tableMetadata = ProtobufUtil.getMutations(request);
List<Mutation> childLinkMutations = Lists.newArrayList();
@@ -2306,8 +2306,16 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso
}
done.run(MetaDataMutationResult.toProto(result));
+ dropTableStats = true;
} finally {
releaseRowLocks(region, locks);
+ if(dropTableStats) {
+ Thread statsDeleteHandler = new Thread(new StatsDeleteHandler(env,
+ loadedTable, tableNamesToDelete, sharedTablesToDelete),
+ "thread-statsdeletehandler");
+ statsDeleteHandler.setDaemon(true);
+ statsDeleteHandler.start();
+ }
}
} catch (Throwable t) {
LOGGER.error("dropTable failed", t);
@@ -2322,6 +2330,50 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso
}
}
+ class StatsDeleteHandler implements Runnable {
+ PTable deletedTable;
+ List<byte[]> physicalTableNames;
+ List<MetaDataProtocol.SharedTableState> sharedTableStates;
+ RegionCoprocessorEnvironment env;
+
+ StatsDeleteHandler(RegionCoprocessorEnvironment env, PTable deletedTable, List<byte[]> physicalTableNames,
+ List<MetaDataProtocol.SharedTableState> sharedTableStates) {
+ this.deletedTable = deletedTable;
+ this.physicalTableNames = physicalTableNames;
+ this.sharedTableStates = sharedTableStates;
+ this.env = env;
+ }
+
+ @Override
+ public void run() {
+ try {
+ User.runAsLoginUser(new PrivilegedExceptionAction<Object>() {
+ @Override
+ public Object run() throws Exception {
+ try (PhoenixConnection connection =
+ QueryUtil.getConnectionOnServer(env.getConfiguration())
+ .unwrap(PhoenixConnection.class)) {
+ try{
+ MetaDataUtil.deleteFromStatsTable(connection, deletedTable,
+ physicalTableNames, sharedTableStates);
+ LOGGER.info("Table stats deleted successfully. "+
+ deletedTable.getPhysicalName().getString());
+ } catch(Throwable t) {
+ LOGGER.warn("Exception while deleting stats of table "
+ + deletedTable.getPhysicalName().getString()
+ + " please check and delete stats manually");
+ }
+ }
+ return null;
+ }
+ });
+ } catch (IOException e) {
+ LOGGER.warn("Exception while deleting stats of table "
+ + deletedTable.getPhysicalName().getString()
+ + " please check and delete stats manually");
+ }
+ }
+ }
private RowLock acquireLock(Region region, byte[] lockKey, List<RowLock> locks) throws IOException {
RowLock rowLock = region.getRowLock(lockKey, false);
if (rowLock == null) {
@@ -3974,8 +4026,8 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso
}
private TableName getParentPhysicalTableName(PTable table) {
- return table
- .getType() == PTableType.VIEW
+ return (table
+ .getType() == PTableType.VIEW || (table.getType() == INDEX && table.getViewIndexId() != null))
? TableName.valueOf(table.getPhysicalName().getBytes())
: table.getType() == PTableType.INDEX
? TableName
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/PhoenixAccessController.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/PhoenixAccessController.java
index 3564ae4..78343d0 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/PhoenixAccessController.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/PhoenixAccessController.java
@@ -109,7 +109,11 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver {
public void preGetTable(ObserverContext<PhoenixMetaDataControllerEnvironment> ctx, String tenantId,
String tableName, TableName physicalTableName) throws IOException {
if (!accessCheckEnabled) { return; }
- requireAccess("GetTable" + tenantId, physicalTableName, Action.READ, Action.EXEC);
+ if(this.execPermissionsCheckEnabled) {
+ requireAccess("GetTable" + tenantId, physicalTableName, Action.READ, Action.EXEC);
+ } else {
+ requireAccess("GetTable" + tenantId, physicalTableName, Action.READ);
+ }
}
@Override
@@ -119,7 +123,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver {
QueryServicesOptions.DEFAULT_PHOENIX_ACLS_ENABLED);
if (!this.accessCheckEnabled) {
LOGGER.warn(
- "PhoenixAccessController has been loaded with authorization checks disabled.");
+ "PhoenixAccessController has been loaded with authorization checks disabled.");
}
this.execPermissionsCheckEnabled = conf.getBoolean(AccessControlConstants.EXEC_PERMISSION_CHECKS_KEY,
AccessControlConstants.DEFAULT_EXEC_PERMISSION_CHECKS);
@@ -242,7 +246,7 @@ public class PhoenixAccessController extends BaseMetaDataEndpointObserver {
// skip check for local index
if (physicalTableName != null && !parentPhysicalTableName.equals(physicalTableName)
&& !MetaDataUtil.isViewIndex(physicalTableName.getNameAsString())) {
- List<Action> actions = Arrays.asList(Action.READ, Action.WRITE, Action.CREATE, Action.ADMIN);
+ List<Action> actions = new ArrayList<>(Arrays.asList(Action.READ, Action.WRITE, Action.CREATE, Action.ADMIN));
if(execPermissionsCheckEnabled) {
actions.add(Action.EXEC);
}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
index c92383d..15e3145 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
@@ -3398,7 +3398,6 @@ public class MetaDataClient {
for (PTable index : table.getIndexes()) {
tableRefs.add(new TableRef(null, index, ts, false));
}
- deleteFromStatsTable(tableRefs, ts);
}
if (!dropMetaData) {
MutationPlan plan = new PostDDLCompiler(connection).compile(tableRefs, null, null,
@@ -3415,32 +3414,6 @@ public class MetaDataClient {
}
}
- private void deleteFromStatsTable(List<TableRef> tableRefs, long ts) throws SQLException {
- boolean isAutoCommit = connection.getAutoCommit();
- try {
- connection.setAutoCommit(true);
- StringBuilder buf = new StringBuilder("DELETE FROM SYSTEM.STATS WHERE PHYSICAL_NAME IN (");
- for (TableRef ref : tableRefs) {
- buf.append("'" + ref.getTable().getPhysicalName().getString() + "',");
- }
- buf.setCharAt(buf.length() - 1, ')');
- if(tableRefs.get(0).getTable().getIndexType()==IndexType.LOCAL) {
- buf.append(" AND COLUMN_FAMILY IN(");
- if (tableRefs.get(0).getTable().getColumnFamilies().isEmpty()) {
- buf.append("'" + QueryConstants.DEFAULT_LOCAL_INDEX_COLUMN_FAMILY + "',");
- } else {
- for(PColumnFamily cf : tableRefs.get(0).getTable().getColumnFamilies()) {
- buf.append("'" + cf.getName().getString() + "',");
- }
- }
- buf.setCharAt(buf.length() - 1, ')');
- }
- connection.createStatement().execute(buf.toString());
- } finally {
- connection.setAutoCommit(isAutoCommit);
- }
- }
-
private MutationCode processMutationResult(String schemaName, String tableName, MetaDataMutationResult result) throws SQLException {
final MutationCode mutationCode = result.getMutationCode();
PName tenantId = connection.getTenantId();
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
index c2a82e5..a5288fc 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
@@ -23,14 +23,7 @@ import static org.apache.phoenix.util.SchemaUtil.getVarChars;
import java.io.IOException;
import java.sql.SQLException;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.NavigableMap;
+import java.util.*;
import com.google.common.annotations.VisibleForTesting;
import org.apache.hadoop.conf.Configuration;
@@ -69,21 +62,10 @@ import org.apache.phoenix.hbase.index.util.VersionUtil;
import org.apache.phoenix.jdbc.PhoenixConnection;
import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
import org.apache.phoenix.query.QueryConstants;
-import org.apache.phoenix.schema.ColumnFamilyNotFoundException;
-import org.apache.phoenix.schema.ColumnNotFoundException;
-import org.apache.phoenix.schema.PColumn;
-import org.apache.phoenix.schema.PColumnFamily;
-import org.apache.phoenix.schema.PName;
-import org.apache.phoenix.schema.PNameFactory;
-import org.apache.phoenix.schema.PTable;
+import org.apache.phoenix.schema.*;
import org.apache.phoenix.schema.PTable.IndexType;
import org.apache.phoenix.schema.PTable.LinkType;
import org.apache.phoenix.schema.PTable.ViewType;
-import org.apache.phoenix.schema.PTableType;
-import org.apache.phoenix.schema.SequenceKey;
-import org.apache.phoenix.schema.SortOrder;
-import org.apache.phoenix.schema.TableNotFoundException;
-import org.apache.phoenix.schema.TableProperty;
import org.apache.phoenix.schema.types.PBoolean;
import org.apache.phoenix.schema.types.PDataType;
import org.apache.phoenix.schema.types.PInteger;
@@ -1004,4 +986,43 @@ public class MetaDataUtil {
}
return col;
}
+
+ public static void deleteFromStatsTable(PhoenixConnection connection,
+ PTable table, List<byte[]> physicalTableNames,
+ List<MetaDataProtocol.SharedTableState> sharedTableStates)
+ throws SQLException {
+ boolean isAutoCommit = connection.getAutoCommit();
+ try {
+ connection.setAutoCommit(true);
+ Set<String> physicalTablesSet = new HashSet<>();
+ Set<String> columnFamilies = new HashSet<>();
+ physicalTablesSet.add(table.getPhysicalName().getString());
+ for(byte[] physicalTableName:physicalTableNames) {
+ physicalTablesSet.add(Bytes.toString(physicalTableName));
+ }
+ for(MetaDataProtocol.SharedTableState s: sharedTableStates) {
+ physicalTablesSet.add(s.getPhysicalNames().get(0).getString());
+ }
+ StringBuilder buf = new StringBuilder("DELETE FROM SYSTEM.STATS WHERE PHYSICAL_NAME IN (");
+ Iterator itr = physicalTablesSet.iterator();
+ while(itr.hasNext()) {
+ buf.append("'" + itr.next() + "',");
+ }
+ buf.setCharAt(buf.length() - 1, ')');
+ if(table.getIndexType()==IndexType.LOCAL) {
+ buf.append(" AND COLUMN_FAMILY IN(");
+ if (table.getColumnFamilies().isEmpty()) {
+ buf.append("'" + QueryConstants.DEFAULT_LOCAL_INDEX_COLUMN_FAMILY + "',");
+ } else {
+ for(PColumnFamily cf : table.getColumnFamilies()) {
+ buf.append("'" + cf.getName().getString() + "',");
+ }
+ }
+ buf.setCharAt(buf.length() - 1, ')');
+ }
+ connection.createStatement().execute(buf.toString());
+ } finally {
+ connection.setAutoCommit(isAutoCommit);
+ }
+ }
}