From ef9f38b68df108816459055bf657aa1546b6845c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?mehmet=20furkan=20=C5=9Fahin?= Date: Fri, 17 Aug 2018 12:24:32 -0700 Subject: [PATCH] ApplyLogRedaction noop func is added --- src/backend/distributed/commands/multi_copy.c | 5 +++-- .../connection/connection_management.c | 7 ++++--- .../distributed/connection/remote_commands.c | 9 ++++++--- .../distributed/executor/multi_client_executor.c | 13 +++++++++++-- .../distributed/executor/multi_server_executor.c | 2 +- .../distributed/master/master_delete_protocol.c | 2 +- .../master/master_modify_multiple_shards.c | 2 +- .../distributed/planner/deparse_shard_query.c | 6 ++++-- .../distributed/planner/insert_select_planner.c | 3 ++- .../distributed/planner/multi_join_order.c | 3 ++- .../distributed/planner/multi_physical_planner.c | 6 ++++-- .../distributed/planner/recursive_planning.c | 15 ++++++++------- .../distributed/test/deparse_shard_query.c | 2 +- .../transaction/distributed_deadlock_detection.c | 3 ++- src/backend/distributed/utils/errormessage.c | 10 ++++++++++ .../worker/worker_data_fetch_protocol.c | 3 ++- .../worker/worker_partition_protocol.c | 2 +- src/include/distributed/errormessage.h | 2 ++ 18 files changed, 65 insertions(+), 30 deletions(-) diff --git a/src/backend/distributed/commands/multi_copy.c b/src/backend/distributed/commands/multi_copy.c index 195d6b157..69a9fc3c9 100644 --- a/src/backend/distributed/commands/multi_copy.c +++ b/src/backend/distributed/commands/multi_copy.c @@ -1240,7 +1240,8 @@ ReportCopyError(MultiConnection *connection, PGresult *result) bool haveDetail = remoteDetail != NULL; ereport(ERROR, (errmsg("%s", remoteMessage), - haveDetail ? errdetail("%s", remoteDetail) : 0)); + haveDetail ? errdetail("%s", ApplyLogRedaction(remoteDetail)) : + 0)); } else { @@ -1250,7 +1251,7 @@ ReportCopyError(MultiConnection *connection, PGresult *result) ereport(ERROR, (errcode(ERRCODE_IO_ERROR), errmsg("failed to complete COPY on %s:%d", connection->hostname, connection->port), - errdetail("%s", remoteMessage))); + errdetail("%s", ApplyLogRedaction(remoteMessage)))); } } diff --git a/src/backend/distributed/connection/connection_management.c b/src/backend/distributed/connection/connection_management.c index c3628f320..38a28dde7 100644 --- a/src/backend/distributed/connection/connection_management.c +++ b/src/backend/distributed/connection/connection_management.c @@ -22,6 +22,7 @@ #include "access/hash.h" #include "commands/dbcommands.h" #include "distributed/connection_management.h" +#include "distributed/errormessage.h" #include "distributed/metadata_cache.h" #include "distributed/hash_helpers.h" #include "distributed/placement_connection.h" @@ -824,9 +825,9 @@ DefaultCitusNoticeProcessor(void *arg, const char *message) char *trimmedMessage = TrimLogLevel(message); char *level = strtok((char *) message, ":"); - ereport(CitusNoticeLogLevel, (errmsg("%s", trimmedMessage), - errdetail("%s from %s:%d", - level, nodeName, nodePort))); + ereport(CitusNoticeLogLevel, + (errmsg("%s", ApplyLogRedaction(trimmedMessage)), + errdetail("%s from %s:%d", level, nodeName, nodePort))); } diff --git a/src/backend/distributed/connection/remote_commands.c b/src/backend/distributed/connection/remote_commands.c index aefecc5f2..fee9c9a16 100644 --- a/src/backend/distributed/connection/remote_commands.c +++ b/src/backend/distributed/connection/remote_commands.c @@ -14,6 +14,7 @@ #include "libpq-fe.h" #include "distributed/connection_management.h" +#include "distributed/errormessage.h" #include "distributed/remote_commands.h" #include "lib/stringinfo.h" #include "miscadmin.h" @@ -253,7 +254,8 @@ ReportConnectionError(MultiConnection *connection, int elevel) ereport(elevel, (errcode(ERRCODE_CONNECTION_FAILURE), errmsg("connection error: %s:%d", nodeName, nodePort), - messageDetail != NULL ? errdetail("%s", messageDetail) : 0)); + messageDetail != NULL ? + errdetail("%s", ApplyLogRedaction(messageDetail)) : 0)); } @@ -295,7 +297,8 @@ ReportResultError(MultiConnection *connection, PGresult *result, int elevel) } ereport(elevel, (errcode(sqlState), errmsg("%s", messagePrimary), - messageDetail ? errdetail("%s", messageDetail) : 0, + messageDetail ? + errdetail("%s", ApplyLogRedaction(messageDetail)) : 0, messageHint ? errhint("%s", messageHint) : 0, messageContext ? errcontext("%s", messageContext) : 0, errcontext("while executing command on %s:%d", @@ -344,7 +347,7 @@ LogRemoteCommand(MultiConnection *connection, const char *command) return; } - ereport(LOG, (errmsg("issuing %s", command), + ereport(LOG, (errmsg("issuing %s", ApplyLogRedaction(command)), errdetail("on server %s:%d", connection->hostname, connection->port))); } diff --git a/src/backend/distributed/executor/multi_client_executor.c b/src/backend/distributed/executor/multi_client_executor.c index 8651ad741..907a37a1c 100644 --- a/src/backend/distributed/executor/multi_client_executor.c +++ b/src/backend/distributed/executor/multi_client_executor.c @@ -419,8 +419,17 @@ MultiClientSendQuery(int32 connectionId, const char *query) if (querySent == 0) { char *errorMessage = pchomp(PQerrorMessage(connection->pgConn)); - ereport(WARNING, (errmsg("could not send remote query \"%s\"", query), - errdetail("Client error: %s", errorMessage))); + + /* + * query might include the user query coming from the taskTracker + * code path, that's why we hash it, too. Otherwise, this code + * path is generally exercised for the kind of errors that + * we cannot send the queries that Citus itself produced. + */ + ereport(WARNING, (errmsg("could not send remote query \"%s\"", + ApplyLogRedaction(query)), + errdetail("Client error: %s", + ApplyLogRedaction(errorMessage)))); success = false; } diff --git a/src/backend/distributed/executor/multi_server_executor.c b/src/backend/distributed/executor/multi_server_executor.c index 61fbaa527..d4f3861bf 100644 --- a/src/backend/distributed/executor/multi_server_executor.c +++ b/src/backend/distributed/executor/multi_server_executor.c @@ -67,7 +67,7 @@ JobExecutorType(DistributedPlan *distributedPlan) ereport(DEBUG2, (errmsg("Plan is router executable"), errdetail("distribution column value: %s", - partitionColumnString))); + ApplyLogRedaction(partitionColumnString)))); } else { diff --git a/src/backend/distributed/master/master_delete_protocol.c b/src/backend/distributed/master/master_delete_protocol.c index 9aa21625b..2da61afeb 100644 --- a/src/backend/distributed/master/master_delete_protocol.c +++ b/src/backend/distributed/master/master_delete_protocol.c @@ -122,7 +122,7 @@ master_apply_delete_command(PG_FUNCTION_ARGS) if (!IsA(queryTreeNode, DeleteStmt)) { ereport(ERROR, (errmsg("query \"%s\" is not a delete statement", - queryString))); + ApplyLogRedaction(queryString)))); } deleteStatement = (DeleteStmt *) queryTreeNode; diff --git a/src/backend/distributed/master/master_modify_multiple_shards.c b/src/backend/distributed/master/master_modify_multiple_shards.c index 4456ab64d..dcf43eb03 100644 --- a/src/backend/distributed/master/master_modify_multiple_shards.c +++ b/src/backend/distributed/master/master_modify_multiple_shards.c @@ -145,7 +145,7 @@ master_modify_multiple_shards(PG_FUNCTION_ARGS) else { ereport(ERROR, (errmsg("query \"%s\" is not a delete, update, or truncate " - "statement", queryString))); + "statement", ApplyLogRedaction(queryString)))); } CheckDistributedTable(relationId); diff --git a/src/backend/distributed/planner/deparse_shard_query.c b/src/backend/distributed/planner/deparse_shard_query.c index 660c4a0ac..e44ceeb04 100644 --- a/src/backend/distributed/planner/deparse_shard_query.c +++ b/src/backend/distributed/planner/deparse_shard_query.c @@ -109,11 +109,13 @@ RebuildQueryStrings(Query *originalQuery, List *taskList) } } - ereport(DEBUG4, (errmsg("query before rebuilding: %s", task->queryString))); + ereport(DEBUG4, (errmsg("query before rebuilding: %s", + ApplyLogRedaction(task->queryString)))); UpdateTaskQueryString(query, relationId, valuesRTE, task); - ereport(DEBUG4, (errmsg("query after rebuilding: %s", task->queryString))); + ereport(DEBUG4, (errmsg("query after rebuilding: %s", + ApplyLogRedaction(task->queryString)))); } } diff --git a/src/backend/distributed/planner/insert_select_planner.c b/src/backend/distributed/planner/insert_select_planner.c index 5ec36b844..3e115c378 100644 --- a/src/backend/distributed/planner/insert_select_planner.c +++ b/src/backend/distributed/planner/insert_select_planner.c @@ -576,7 +576,8 @@ RouterModifyTaskForShardInterval(Query *originalQuery, ShardInterval *shardInter /* and generate the full query string */ deparse_shard_query(copiedQuery, distributedTableId, shardInterval->shardId, queryString); - ereport(DEBUG2, (errmsg("distributed statement: %s", queryString->data))); + ereport(DEBUG2, (errmsg("distributed statement: %s", + ApplyLogRedaction(queryString->data)))); modifyTask = CreateBasicTask(jobId, taskIdIndex, MODIFY_TASK, queryString->data); modifyTask->dependedTaskList = NULL; diff --git a/src/backend/distributed/planner/multi_join_order.c b/src/backend/distributed/planner/multi_join_order.c index ee1504a86..8a2eedc63 100644 --- a/src/backend/distributed/planner/multi_join_order.c +++ b/src/backend/distributed/planner/multi_join_order.c @@ -714,7 +714,8 @@ PrintJoinOrderList(List *joinOrder) } } - ereport(LOG, (errmsg("join order: %s", printBuffer->data))); + ereport(LOG, (errmsg("join order: %s", + ApplyLogRedaction(printBuffer->data)))); } diff --git a/src/backend/distributed/planner/multi_physical_planner.c b/src/backend/distributed/planner/multi_physical_planner.c index b6a0682ca..35766de01 100644 --- a/src/backend/distributed/planner/multi_physical_planner.c +++ b/src/backend/distributed/planner/multi_physical_planner.c @@ -2418,7 +2418,8 @@ QueryPushdownTaskCreate(Query *originalQuery, int shardIndex, taskType == SQL_TASK) { pg_get_query_def(taskQuery, queryString); - ereport(DEBUG4, (errmsg("distributed statement: %s", queryString->data))); + ereport(DEBUG4, (errmsg("distributed statement: %s", + ApplyLogRedaction(queryString->data)))); subqueryTask->queryString = queryString->data; } @@ -2698,7 +2699,8 @@ SqlTaskList(Job *job) /* log the query string we generated */ ereport(DEBUG4, (errmsg("generated sql query for task %d", sqlTask->taskId), - errdetail("query string: \"%s\"", sqlQueryString->data))); + errdetail("query string: \"%s\"", + ApplyLogRedaction(sqlQueryString->data)))); sqlTask->anchorShardId = INVALID_SHARD_ID; if (anchorRangeTableBasedAssignment) diff --git a/src/backend/distributed/planner/recursive_planning.c b/src/backend/distributed/planner/recursive_planning.c index cc444c206..f5412d7bc 100644 --- a/src/backend/distributed/planner/recursive_planning.c +++ b/src/backend/distributed/planner/recursive_planning.c @@ -218,7 +218,7 @@ GenerateSubplansForSubqueriesAndCTEs(uint64 planId, Query *originalQuery, ereport(DEBUG1, (errmsg( "Plan " UINT64_FORMAT " query after replacing subqueries and CTEs: %s", planId, - subPlanString->data))); + ApplyLogRedaction(subPlanString->data)))); } return context.subPlanList; @@ -715,9 +715,10 @@ RecursivelyPlanCTEs(Query *query, RecursivePlanningContext *planningContext) { StringInfo subPlanString = makeStringInfo(); pg_get_query_def(subquery, subPlanString); - ereport(DEBUG1, (errmsg("generating subplan " UINT64_FORMAT "_%u for " - "CTE %s: %s", - planId, subPlanId, cteName, subPlanString->data))); + ereport(DEBUG1, (errmsg("generating subplan " UINT64_FORMAT + "_%u for CTE %s: %s", planId, subPlanId, + cteName, + ApplyLogRedaction(subPlanString->data)))); } /* build a sub plan for the CTE */ @@ -1120,9 +1121,9 @@ RecursivelyPlanSubquery(Query *subquery, RecursivePlanningContext *planningConte pg_get_query_def(debugQuery, subqueryString); - ereport(DEBUG1, (errmsg("generating subplan " UINT64_FORMAT "_%u for " - "subquery %s", - planId, subPlanId, subqueryString->data))); + ereport(DEBUG1, (errmsg("generating subplan " UINT64_FORMAT + "_%u for subquery %s", planId, subPlanId, + ApplyLogRedaction(subqueryString->data)))); } /* finally update the input subquery to point the result query */ diff --git a/src/backend/distributed/test/deparse_shard_query.c b/src/backend/distributed/test/deparse_shard_query.c index 55c75de4a..7c3f8eeda 100644 --- a/src/backend/distributed/test/deparse_shard_query.c +++ b/src/backend/distributed/test/deparse_shard_query.c @@ -76,7 +76,7 @@ deparse_shard_query_test(PG_FUNCTION_ARGS) deparse_shard_query(query, InvalidOid, 0, buffer); - elog(INFO, "query: %s", buffer->data); + elog(INFO, "query: %s", ApplyLogRedaction(buffer->data)); } } diff --git a/src/backend/distributed/transaction/distributed_deadlock_detection.c b/src/backend/distributed/transaction/distributed_deadlock_detection.c index 0bba3a6cd..ab32cd51f 100644 --- a/src/backend/distributed/transaction/distributed_deadlock_detection.c +++ b/src/backend/distributed/transaction/distributed_deadlock_detection.c @@ -17,6 +17,7 @@ #include "access/hash.h" #include "distributed/backend_data.h" #include "distributed/distributed_deadlock_detection.h" +#include "distributed/errormessage.h" #include "distributed/hash_helpers.h" #include "distributed/listutils.h" #include "distributed/lock_graph.h" @@ -673,7 +674,7 @@ LogDistributedDeadlockDebugMessage(const char *errorMessage) } ereport(LOG, (errmsg("[%s] %s", timestamptz_to_str(GetCurrentTimestamp()), - errorMessage))); + ApplyLogRedaction(errorMessage)))); } diff --git a/src/backend/distributed/utils/errormessage.c b/src/backend/distributed/utils/errormessage.c index f2b69986e..e0ba27771 100644 --- a/src/backend/distributed/utils/errormessage.c +++ b/src/backend/distributed/utils/errormessage.c @@ -12,6 +12,16 @@ #include "distributed/errormessage.h" +/* + * ApplyLogRedaction is only supported in Citus Enterprise + */ +char * +ApplyLogRedaction(const char *text) +{ + return (char *) text; +} + + /* * DeferredErrorInternal is a helper function for DeferredError(). */ diff --git a/src/backend/distributed/worker/worker_data_fetch_protocol.c b/src/backend/distributed/worker/worker_data_fetch_protocol.c index 0a760f192..a986b7876 100644 --- a/src/backend/distributed/worker/worker_data_fetch_protocol.c +++ b/src/backend/distributed/worker/worker_data_fetch_protocol.c @@ -710,7 +710,8 @@ ParseTreeRawStmt(const char *ddlCommand) /* log immediately if dictated by log statement */ if (check_log_statement(parseTreeList)) { - ereport(LOG, (errmsg("statement: %s", ddlCommand), errhidestmt(true))); + ereport(LOG, (errmsg("statement: %s", ApplyLogRedaction(ddlCommand)), + errhidestmt(true))); } parseTreeCount = list_length(parseTreeList); diff --git a/src/backend/distributed/worker/worker_partition_protocol.c b/src/backend/distributed/worker/worker_partition_protocol.c index 3a0ba7542..9be7dafda 100644 --- a/src/backend/distributed/worker/worker_partition_protocol.c +++ b/src/backend/distributed/worker/worker_partition_protocol.c @@ -923,7 +923,7 @@ FilterAndPartitionTable(const char *filterQuery, if (queryPortal == NULL) { ereport(ERROR, (errmsg("could not open implicit cursor for query \"%s\"", - filterQuery))); + ApplyLogRedaction(filterQuery)))); } rowOutputState = InitRowOutputState(); diff --git a/src/include/distributed/errormessage.h b/src/include/distributed/errormessage.h index ffc9ae0f0..4626da6b6 100644 --- a/src/include/distributed/errormessage.h +++ b/src/include/distributed/errormessage.h @@ -70,3 +70,5 @@ DeferredErrorMessage * DeferredErrorInternal(int code, const char *message, cons void RaiseDeferredErrorInternal(DeferredErrorMessage *error, int elevel); #endif + +extern char * ApplyLogRedaction(const char *text);