Skip to content

Commit e045768

Browse files
dengzhhu653dengzhhu653
authored
HIVE-29330: HiveMetaStoreAuthorizer creating new HiveConf per thread increases overhead (#6205)
1 parent ee7138b commit e045768

File tree

10 files changed

+70
-65
lines changed

10 files changed

+70
-65
lines changed

‎common/src/java/org/apache/hadoop/hive/conf/HiveConf.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6551,6 +6551,19 @@ public HiveConf(Configuration other, Class<?> cls) {
65516551
initialize(cls);
65526552
}
65536553

6554+
/**
6555+
* For internal use only, assumed the "other" has loaded all properties that intend to use
6556+
* and want to cast it to a HiveConf without extra re-loading the source file.
6557+
* @param other The Configuration whose properties are to be wrapped by this HiveConf.
6558+
*/
6559+
private HiveConf(Configuration other) {
6560+
super(other);
6561+
setupRestrictList();
6562+
hiddenSet.addAll(HiveConfUtil.getHiddenSet(other));
6563+
lockedSet.addAll(HiveConfUtil.getLockedSet(other));
6564+
origProp = getProperties(other);
6565+
}
6566+
65546567
/**
65556568
* Copy constructor
65566569
*/
@@ -7313,4 +7326,19 @@ public void syncFromConf(HiveConf conf) {
73137326
set(e.getKey(), e.getValue());
73147327
}
73157328
}
7329+
7330+
/**
7331+
* Sometimes if the configuration contains all the information we want,
7332+
* but want to cast it to a HiveConf, without loading the props from the
7333+
* source file again, which is wasteful and might cost dozens of milliseconds.
7334+
* @param configuration The original configuration
7335+
* @return A HiveConf wrapping on the original configuration
7336+
*/
7337+
public static HiveConf cloneConf(Configuration configuration) {
7338+
if (configuration instanceof HiveConf config) {
7339+
return new HiveConf(config);
7340+
} else {
7341+
return new HiveConf(configuration);
7342+
}
7343+
}
73167344
}

‎ql/src/java/org/apache/hadoop/hive/ql/ddl/table/execute/AlterTableExecuteAnalyzer.java‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
import org.apache.hadoop.hive.ql.plan.ExprNodeConstantDesc;
4747
import org.apache.hadoop.hive.ql.plan.ExprNodeDesc;
4848
import org.apache.hadoop.hive.ql.plan.PlanUtils;
49-
import org.apache.hadoop.hive.ql.session.SessionState;
5049

5150
import java.time.ZoneId;
5251
import java.util.List;
@@ -109,14 +108,14 @@ protected void analyzeCommand(TableName tableName, Map<String, String> partition
109108
rootTasks.add(TaskFactory.get(new DDLWork(getInputs(), getOutputs(), desc)));
110109
}
111110

112-
private static AlterTableExecuteDesc getCherryPickDesc(TableName tableName, Map<String, String> partitionSpec,
111+
private AlterTableExecuteDesc getCherryPickDesc(TableName tableName, Map<String, String> partitionSpec,
113112
ASTNode childNode) throws SemanticException {
114113
long snapshotId = Long.parseLong(childNode.getText());
115114
AlterTableExecuteSpec spec = new AlterTableExecuteSpec(CHERRY_PICK, new CherryPickSpec(snapshotId));
116115
return new AlterTableExecuteDesc(tableName, partitionSpec, spec);
117116
}
118117

119-
private static AlterTableExecuteDesc getFastForwardDesc(TableName tableName, Map<String, String> partitionSpec,
118+
private AlterTableExecuteDesc getFastForwardDesc(TableName tableName, Map<String, String> partitionSpec,
120119
ASTNode command) throws SemanticException {
121120
String branchName;
122121
String targetBranchName;
@@ -135,24 +134,22 @@ private static AlterTableExecuteDesc getFastForwardDesc(TableName tableName, Map
135134
return new AlterTableExecuteDesc(tableName, partitionSpec, spec);
136135
}
137136

138-
private static AlterTableExecuteDesc getSetCurrentSnapshotDesc(TableName tableName, Map<String, String> partitionSpec,
137+
private AlterTableExecuteDesc getSetCurrentSnapshotDesc(TableName tableName, Map<String, String> partitionSpec,
139138
ASTNode childNode) throws SemanticException {
140139
AlterTableExecuteSpec<AlterTableExecuteSpec.SetCurrentSnapshotSpec> spec =
141140
new AlterTableExecuteSpec(SET_CURRENT_SNAPSHOT,
142141
new AlterTableExecuteSpec.SetCurrentSnapshotSpec(childNode.getText()));
143142
return new AlterTableExecuteDesc(tableName, partitionSpec, spec);
144143
}
145144

146-
private static AlterTableExecuteDesc getExpireSnapshotDesc(TableName tableName, Map<String, String> partitionSpec,
145+
private AlterTableExecuteDesc getExpireSnapshotDesc(TableName tableName, Map<String, String> partitionSpec,
147146
List<Node> children, HiveConf conf) throws SemanticException {
148147
AlterTableExecuteSpec<ExpireSnapshotsSpec> spec;
149148
if (children.size() == 1) {
150149
spec = new AlterTableExecuteSpec(EXPIRE_SNAPSHOT, null);
151150
return new AlterTableExecuteDesc(tableName, partitionSpec, spec);
152151
}
153-
ZoneId timeZone = SessionState.get() == null ?
154-
new HiveConf().getLocalTimeZone() :
155-
SessionState.get().getConf().getLocalTimeZone();
152+
ZoneId timeZone = conf.getLocalTimeZone();
156153
ASTNode firstNode = (ASTNode) children.get(1);
157154
String firstNodeText = PlanUtils.stripQuotes(firstNode.getText().trim());
158155
if (firstNode.getType() == KW_RETAIN) {
@@ -176,7 +173,7 @@ private static AlterTableExecuteDesc getExpireSnapshotDesc(TableName tableName,
176173
return new AlterTableExecuteDesc(tableName, partitionSpec, spec);
177174
}
178175

179-
private static String getTimeStampString(HiveConf conf, ASTNode node, String nodeText) throws SemanticException {
176+
private String getTimeStampString(HiveConf conf, ASTNode node, String nodeText) throws SemanticException {
180177
if (node.getChildCount() > 0) {
181178
QueryState queryState = new QueryState.Builder().withGenerateNewQueryId(false).withHiveConf(conf).build();
182179
SemanticAnalyzer sem = (SemanticAnalyzer) SemanticAnalyzerFactory.get(queryState, node);
@@ -190,14 +187,12 @@ private static String getTimeStampString(HiveConf conf, ASTNode node, String nod
190187
return nodeText;
191188
}
192189

193-
private static AlterTableExecuteDesc getRollbackDesc(TableName tableName, Map<String, String> partitionSpec,
190+
private AlterTableExecuteDesc getRollbackDesc(TableName tableName, Map<String, String> partitionSpec,
194191
ASTNode childNode) throws SemanticException {
195192
AlterTableExecuteSpec<RollbackSpec> spec;
196193
// the child must be the rollback parameter
197194
if (childNode.getType() == HiveParser.StringLiteral) {
198-
ZoneId timeZone = SessionState.get() == null ?
199-
new HiveConf().getLocalTimeZone() :
200-
SessionState.get().getConf().getLocalTimeZone();
195+
ZoneId timeZone = conf.getLocalTimeZone();
201196
TimestampTZ time = TimestampTZUtil.parse(PlanUtils.stripQuotes(childNode.getText()), timeZone);
202197
spec = new AlterTableExecuteSpec(ROLLBACK, new RollbackSpec(TIME, time.toEpochMilli()));
203198
} else {
@@ -206,7 +201,7 @@ private static AlterTableExecuteDesc getRollbackDesc(TableName tableName, Map<St
206201
return new AlterTableExecuteDesc(tableName, partitionSpec, spec);
207202
}
208203

209-
private static AlterTableExecuteDesc getDeleteOrphanFilesDesc(TableName tableName, Map<String, String> partitionSpec,
204+
private AlterTableExecuteDesc getDeleteOrphanFilesDesc(TableName tableName, Map<String, String> partitionSpec,
210205
List<Node> children) throws SemanticException {
211206

212207
long time = System.currentTimeMillis() - TimeUnit.DAYS.toMillis(3);
@@ -217,11 +212,9 @@ private static AlterTableExecuteDesc getDeleteOrphanFilesDesc(TableName tableNam
217212
return new AlterTableExecuteDesc(tableName, partitionSpec, spec);
218213
}
219214

220-
private static long getTimeStampMillis(ASTNode childNode) {
215+
private long getTimeStampMillis(ASTNode childNode) {
221216
String childNodeText = PlanUtils.stripQuotes(childNode.getText());
222-
ZoneId timeZone = SessionState.get() == null ?
223-
new HiveConf().getLocalTimeZone() :
224-
SessionState.get().getConf().getLocalTimeZone();
217+
ZoneId timeZone = conf.getLocalTimeZone();
225218
TimestampTZ time = TimestampTZUtil.parse(PlanUtils.stripQuotes(childNodeText), timeZone);
226219
return time.toEpochMilli();
227220
}

‎ql/src/java/org/apache/hadoop/hive/ql/ddl/table/snapshotref/AlterTableCreateSnapshotRefAnalyzer.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.apache.hadoop.hive.common.TableName;
2626
import org.apache.hadoop.hive.common.type.TimestampTZ;
2727
import org.apache.hadoop.hive.common.type.TimestampTZUtil;
28-
import org.apache.hadoop.hive.conf.HiveConf;
2928
import org.apache.hadoop.hive.ql.QueryState;
3029
import org.apache.hadoop.hive.ql.ddl.DDLUtils;
3130
import org.apache.hadoop.hive.ql.ddl.DDLWork;
@@ -39,7 +38,6 @@
3938
import org.apache.hadoop.hive.ql.parse.AlterTableSnapshotRefSpec;
4039
import org.apache.hadoop.hive.ql.parse.HiveParser;
4140
import org.apache.hadoop.hive.ql.parse.SemanticException;
42-
import org.apache.hadoop.hive.ql.session.SessionState;
4341

4442
public abstract class AlterTableCreateSnapshotRefAnalyzer extends AbstractAlterTableAnalyzer {
4543
protected AlterTableType alterTableType;
@@ -72,8 +70,7 @@ protected void analyzeCommand(TableName tableName, Map<String, String> partition
7270
snapshotId = Long.parseLong(childNode.getChild(0).getText());
7371
break;
7472
case HiveParser.TOK_AS_OF_TIME:
75-
ZoneId timeZone = SessionState.get() == null ? new HiveConf().getLocalTimeZone() :
76-
SessionState.get().getConf().getLocalTimeZone();
73+
ZoneId timeZone = conf.getLocalTimeZone();
7774
TimestampTZ ts = TimestampTZUtil.parse(stripQuotes(childNode.getChild(0).getText()), timeZone);
7875
asOfTime = ts.toEpochMilli();
7976
break;

‎ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewUtils.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ public static RelOptMaterialization createCTEMaterialization(String viewName, Re
569569
hiveTable.setMaterializedTable(true);
570570
RelOptHiveTable optTable =
571571
new RelOptHiveTable(null, cluster.getTypeFactory(), fullName, body.getRowType(), hiveTable, columns,
572-
Collections.emptyList(), Collections.emptyList(), new HiveConf(), new QueryTables(true), new HashMap<>(),
572+
Collections.emptyList(), Collections.emptyList(), conf, new QueryTables(true), new HashMap<>(),
573573
new HashMap<>(), new AtomicInteger());
574574
optTable.setRowCount(cluster.getMetadataQuery().getRowCount(body));
575575
final TableScan scan =

‎ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/GenMRSkewJoinProcessor.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,7 @@ public static void processSkewJoin(JoinOperator joinOp,
319319
}
320320
mapJoinOp.setChildOperators(childOps);
321321

322-
HiveConf jc = new HiveConf(parseCtx.getConf(),
323-
GenMRSkewJoinProcessor.class);
322+
HiveConf jc = new HiveConf(parseCtx.getConf());
324323

325324
newPlan.setNumMapTasks(HiveConf
326325
.getIntVar(jc, HiveConf.ConfVars.HIVE_SKEWJOIN_MAPJOIN_NUM_MAP_TASK));

‎ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveMetastoreClientFactoryImpl.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020

2121
import org.apache.hadoop.hive.common.classification.InterfaceAudience.Private;
22+
import org.apache.hadoop.hive.conf.HiveConf;
2223
import org.apache.hadoop.hive.metastore.IMetaStoreClient;
2324
import org.apache.hadoop.hive.metastore.api.MetaException;
2425
import org.apache.hadoop.hive.ql.metadata.Hive;
@@ -29,11 +30,17 @@
2930
@Private
3031
public class HiveMetastoreClientFactoryImpl implements HiveMetastoreClientFactory{
3132

33+
private final HiveConf hiveConf;
34+
35+
public HiveMetastoreClientFactoryImpl(HiveConf conf) {
36+
this.hiveConf = conf;
37+
}
38+
3239
@Override
3340
public IMetaStoreClient getHiveMetastoreClient() throws HiveAuthzPluginException {
3441
String errMsg = "Error getting metastore client";
3542
try {
36-
return Hive.get().getMSC();
43+
return Hive.get(hiveConf, false).getMSC();
3744
} catch (MetaException e) {
3845
throw new HiveAuthzPluginException(errMsg, e);
3946
} catch (HiveException e) {

‎ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/HiveMetaStoreAuthorizer.java‎

Lines changed: 15 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,8 @@
4040
import org.apache.hadoop.hive.metastore.api.PartitionSpec;
4141
import org.apache.hadoop.hive.metastore.api.TableMeta;
4242
import org.apache.hadoop.hive.metastore.utils.MetaStoreUtils;
43-
import org.apache.hadoop.hive.ql.metadata.HiveException;
4443
import org.apache.hadoop.hive.ql.metadata.HiveUtils;
45-
import org.apache.hadoop.hive.ql.security.HiveMetastoreAuthenticationProvider;
44+
import org.apache.hadoop.hive.ql.security.HiveAuthenticationProvider;
4645
import static org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrivilegeObjectUtils.TablePrivilegeLookup;
4746
import org.apache.hadoop.hive.ql.security.authorization.plugin.metastore.events.*;
4847
import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAccessControlException;
@@ -82,12 +81,7 @@ public class HiveMetaStoreAuthorizer extends MetaStorePreEventListener implement
8281
/**
8382
* The client configuration.
8483
*/
85-
private static final ThreadLocal<Map<String, Object>> cConfig = new ThreadLocal<Map<String, Object>>() {
86-
@Override
87-
protected Map<String, Object> initialValue() {
88-
return null;
89-
}
90-
};
84+
private static final ThreadLocal<Map<String, Object>> cConfig = ThreadLocal.withInitial(() -> null);
9185

9286
public static void setClientConfig(Map<String, Object> map) {
9387
cConfig.set(map);
@@ -97,24 +91,7 @@ public static Map<String, Object> getClientConfig() {
9791
return cConfig.get();
9892
}
9993

100-
private static final ThreadLocal<Configuration> tConfig = new ThreadLocal<Configuration>() {
101-
102-
@Override
103-
protected Configuration initialValue() {
104-
return null;
105-
}
106-
};
107-
108-
private static final ThreadLocal<HiveMetastoreAuthenticationProvider> tAuthenticator = new ThreadLocal<HiveMetastoreAuthenticationProvider>() {
109-
@Override
110-
protected HiveMetastoreAuthenticationProvider initialValue() {
111-
try {
112-
return (HiveMetastoreAuthenticationProvider) HiveUtils.getAuthenticator(tConfig.get(), HiveConf.ConfVars.HIVE_METASTORE_AUTHENTICATOR_MANAGER);
113-
} catch (HiveException excp) {
114-
throw new IllegalStateException("Authentication provider instantiation failure", excp);
115-
}
116-
}
117-
};
94+
private static final ThreadLocal<HiveAuthenticationProvider> tAuthenticator = ThreadLocal.withInitial(() -> null);
11895

11996
public HiveMetaStoreAuthorizer(Configuration config) {
12097
super(config);
@@ -627,18 +604,20 @@ HiveMetaStoreAuthzInfo buildAuthzContext(PreEventContext preEventContext) throws
627604

628605
HiveAuthorizer createHiveMetaStoreAuthorizer() throws Exception {
629606
HiveAuthorizer ret = null;
630-
HiveConf hiveConf = (HiveConf)tConfig.get();
631-
if(hiveConf == null){
632-
HiveConf hiveConf1 = new HiveConf(super.getConf(), HiveConf.class);
633-
tConfig.set(hiveConf1);
634-
hiveConf = hiveConf1;
635-
}
607+
// If it's insides the HMS, getConf() should have all properties in hive-site.xml,
608+
// otherwise it at least contains the information to talk with the HMS,
609+
// as the call is triggered from client as a filter hook.
610+
HiveConf hiveConf = HiveConf.cloneConf(getConf());
611+
636612
HiveAuthorizerFactory authorizerFactory =
637613
HiveUtils.getAuthorizerFactory(hiveConf, HiveConf.ConfVars.HIVE_AUTHORIZATION_MANAGER);
638-
639614
if (authorizerFactory != null) {
640-
HiveMetastoreAuthenticationProvider authenticator = tAuthenticator.get();
641-
615+
HiveAuthenticationProvider authenticator = tAuthenticator.get();
616+
if (authenticator == null) {
617+
authenticator = HiveUtils.getAuthenticator(hiveConf,
618+
HiveConf.ConfVars.HIVE_METASTORE_AUTHENTICATOR_MANAGER);
619+
tAuthenticator.set(authenticator);
620+
}
642621
authenticator.setConf(hiveConf);
643622

644623
HiveAuthzSessionContext.Builder authzContextBuilder = new HiveAuthzSessionContext.Builder();
@@ -649,7 +628,7 @@ HiveAuthorizer createHiveMetaStoreAuthorizer() throws Exception {
649628
HiveAuthzSessionContext authzSessionContext = authzContextBuilder.build();
650629

651630
ret = authorizerFactory
652-
.createHiveAuthorizer(new HiveMetastoreClientFactoryImpl(), hiveConf, authenticator, authzSessionContext);
631+
.createHiveAuthorizer(new HiveMetastoreClientFactoryImpl(hiveConf), hiveConf, authenticator, authzSessionContext);
653632
}
654633

655634
return ret;

‎ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1026,7 +1026,7 @@ private synchronized void setupAuth() {
10261026
: CLIENT_TYPE.HIVECLI);
10271027
authzContextBuilder.setSessionString(getSessionId());
10281028

1029-
authorizerV2 = authorizerFactory.createHiveAuthorizer(new HiveMetastoreClientFactoryImpl(),
1029+
authorizerV2 = authorizerFactory.createHiveAuthorizer(new HiveMetastoreClientFactoryImpl(getSessionConf()),
10301030
sessionConf, authenticator, authzContextBuilder.build());
10311031
setAuthorizerV2Config();
10321032

‎ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorThread.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public void setConf(Configuration configuration) {
6161
// HiveConf is moved to the standalone metastore.
6262
//clone the conf - compactor needs to set properties in it which we don't
6363
// want to bleed into the caller
64-
conf = new HiveConf(configuration, HiveConf.class);
64+
conf = HiveConf.cloneConf(configuration);
6565
}
6666

6767
@Override

‎ql/src/java/org/apache/hadoop/hive/ql/udf/UDFUtils.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@
2828

2929
public class UDFUtils {
3030

31+
private static final HiveConf HIVE_CONF = new HiveConf();
32+
3133
public static TimestampTZ getTimestampTZFromTimestamp(Timestamp timestamp) {
3234
ZoneId zone = ((SessionState.get() == null) ?
33-
new HiveConf().getLocalTimeZone() : SessionState.get().getConf().getLocalTimeZone());
35+
HIVE_CONF.getLocalTimeZone() : SessionState.get().getConf().getLocalTimeZone());
3436
return TimestampTZUtil.convert(timestamp, zone);
3537
}
3638
}

0 commit comments

Comments
 (0)