stoty commented on a change in pull request #1210:
URL: https://github.com/apache/phoenix/pull/1210#discussion_r621016404
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
##########
@@ -349,12 +349,14 @@ public ConnectionInfo normalize(ReadOnlyProps props,
Properties info) throws SQL
}
if(principal == null){
if (!isConnectionless) {
- principal = props.get(QueryServices.HBASE_CLIENT_PRINCIPAL);
+ principal =
props.get(QueryServices.HBASE_CLIENT_PRINCIPAL);
+// principal = principal == null ?
info.getProperty(QueryServices.HBASE_CLIENT_PRINCIPAL) : principal;
Review comment:
Remove commented out code
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/log/QueryLoggerDisruptor.java
##########
@@ -74,12 +74,33 @@ public Thread newThread(Runnable r) {
final ExceptionHandler<RingBufferEvent> errorHandler = new
QueryLoggerDefaultExceptionHandler();
disruptor.setDefaultExceptionHandler(errorHandler);
- final QueryLogDetailsEventHandler[] handlers = { new
QueryLogDetailsEventHandler(configuration) };
- disruptor.handleEventsWith(handlers);
+ /**
+ * if LOG_HANDLER_COUNT is 1 it will work as the previous
implementation
+ * if LOG_HANDLER_COUNT is 2 or more then Multi Thread
+ */
+ int handlerCount = configuration.getInt(
+ QueryServices.LOG_HANDLER_COUNT,
DEFAULT_AUDIT_LOGGER_PROCESS_COUNT);
+
+ if (handlerCount <= 0){
+ LOGGER.error("Audit Log Handler Count must be greater than 0." +
+ "change to default value, input : " + handlerCount);
+ handlerCount = DEFAULT_AUDIT_LOGGER_PROCESS_COUNT;
+ }
+
+ if (handlerCount == 1) {
Review comment:
Can't we just use a workhandler pool with a single worker ?
I think we need not keep QueryLogDetailsEventHandler.
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java
##########
@@ -1855,6 +1893,26 @@ public MutationPlan compileMutation(String sql) throws
SQLException {
return compileMutation(stmt, sql);
}
+ public boolean checkIgnoreQueryAudit(CompilableStatement stmt, String sql)
{
+ boolean needIgnore = false;
+
+ TableName tableName = null;
+ if (stmt instanceof ExecutableUpsertStatement) {
+ tableName = ((ExecutableUpsertStatement)stmt).getTable().getName();
+ } else if (stmt instanceof ExecutableDeleteStatement) {
+ tableName = ((ExecutableDeleteStatement)stmt).getTable().getName();
+ } else if (stmt instanceof ExecutableAddColumnStatement) {
+ tableName =
((ExecutableAddColumnStatement)stmt).getTable().getName();
+ }
+
+ if (tableName != null && PhoenixDatabaseMetaData.SYSTEM_CATALOG_SCHEMA
+ .equals(tableName.getSchemaName())) {
+ needIgnore = true;
+ }
+
+ return needIgnore;
+ }
+
public QueryLogger createQueryLogger(CompilableStatement stmt, String sql)
throws SQLException {
if (connection.getLogLevel() == LogLevel.OFF) {
Review comment:
This looks suspect.
Shouldn't we check the audit log status too ?
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java
##########
@@ -1922,7 +1983,7 @@ public boolean execute(String sql) throws SQLException {
throw new
SQLExceptionInfo.Builder(SQLExceptionCode.EXECUTE_UPDATE_WITH_NON_EMPTY_BATCH)
.build().buildException();
}
- executeMutation(stmt);
+ executeMutation(stmt, createQueryLogger(stmt,sql));
Review comment:
nit:space after comma
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java
##########
@@ -1903,7 +1964,7 @@ public int executeUpdate(String sql) throws SQLException {
throw new
SQLExceptionInfo.Builder(SQLExceptionCode.EXECUTE_UPDATE_WITH_NON_EMPTY_BATCH)
.build().buildException();
}
- int updateCount = executeMutation(stmt);
+ int updateCount = executeMutation(stmt, createQueryLogger(stmt,sql));
Review comment:
nit: space after comma
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java
##########
@@ -1855,6 +1893,26 @@ public MutationPlan compileMutation(String sql) throws
SQLException {
return compileMutation(stmt, sql);
}
+ public boolean checkIgnoreQueryAudit(CompilableStatement stmt, String sql)
{
Review comment:
We could move the check for ExecutableSelectStatement here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
