Skip to content

Commit 7e12a93

Browse files
NJrslvtuhaihe
authored andcommitted
[gp_stats_collector] Build by default with extension disabled via GUCs
Enable building gp_stats_collector by default in configure. Add missing check in verify_query() to ensure the extension does not execute main code while disabled. Always verify protobuf version once the shared library is preloaded.
1 parent 8681303 commit 7e12a93

6 files changed

Lines changed: 16 additions & 16 deletions

File tree

.github/workflows/build-cloudberry-rocky8.yml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -544,11 +544,10 @@ jobs:
544544
if: needs.check-skip.outputs.should_skip != 'true'
545545
env:
546546
SRC_DIR: ${{ github.workspace }}
547-
CONFIGURE_EXTRA_OPTS: --with-gp-stats-collector
548547
run: |
549548
set -eo pipefail
550549
chmod +x "${SRC_DIR}"/devops/build/automation/cloudberry/scripts/configure-cloudberry.sh
551-
if ! time su - gpadmin -c "cd ${SRC_DIR} && SRC_DIR=${SRC_DIR} ENABLE_DEBUG=${{ env.ENABLE_DEBUG }} CONFIGURE_EXTRA_OPTS=${{ env.CONFIGURE_EXTRA_OPTS }} ${SRC_DIR}/devops/build/automation/cloudberry/scripts/configure-cloudberry.sh"; then
550+
if ! time su - gpadmin -c "cd ${SRC_DIR} && SRC_DIR=${SRC_DIR} ENABLE_DEBUG=${{ env.ENABLE_DEBUG }} ${SRC_DIR}/devops/build/automation/cloudberry/scripts/configure-cloudberry.sh"; then
552551
echo "::error::Configure script failed"
553552
exit 1
554553
fi

.github/workflows/build-cloudberry.yml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,11 +539,10 @@ jobs:
539539
if: needs.check-skip.outputs.should_skip != 'true'
540540
env:
541541
SRC_DIR: ${{ github.workspace }}
542-
CONFIGURE_EXTRA_OPTS: --with-gp-stats-collector
543542
run: |
544543
set -eo pipefail
545544
chmod +x "${SRC_DIR}"/devops/build/automation/cloudberry/scripts/configure-cloudberry.sh
546-
if ! time su - gpadmin -c "cd ${SRC_DIR} && SRC_DIR=${SRC_DIR} ENABLE_DEBUG=${{ env.ENABLE_DEBUG }} CONFIGURE_EXTRA_OPTS=${{ env.CONFIGURE_EXTRA_OPTS }} ${SRC_DIR}/devops/build/automation/cloudberry/scripts/configure-cloudberry.sh"; then
545+
if ! time su - gpadmin -c "cd ${SRC_DIR} && SRC_DIR=${SRC_DIR} ENABLE_DEBUG=${{ env.ENABLE_DEBUG }} ${SRC_DIR}/devops/build/automation/cloudberry/scripts/configure-cloudberry.sh"; then
547546
echo "::error::Configure script failed"
548547
exit 1
549548
fi

.github/workflows/build-deb-cloudberry.yml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -452,14 +452,13 @@ jobs:
452452
shell: bash
453453
env:
454454
SRC_DIR: ${{ github.workspace }}
455-
CONFIGURE_EXTRA_OPTS: --with-gp-stats-collector
456455
run: |
457456
set -eo pipefail
458457
459458
export BUILD_DESTINATION=${SRC_DIR}/debian/build
460459
461460
chmod +x "${SRC_DIR}"/devops/build/automation/cloudberry/scripts/configure-cloudberry.sh
462-
if ! time su - gpadmin -c "cd ${SRC_DIR} && SRC_DIR=${SRC_DIR} ENABLE_DEBUG=${{ env.ENABLE_DEBUG }} CONFIGURE_EXTRA_OPTS=${{ env.CONFIGURE_EXTRA_OPTS }} BUILD_DESTINATION=${BUILD_DESTINATION} ${SRC_DIR}/devops/build/automation/cloudberry/scripts/configure-cloudberry.sh"; then
461+
if ! time su - gpadmin -c "cd ${SRC_DIR} && SRC_DIR=${SRC_DIR} ENABLE_DEBUG=${{ env.ENABLE_DEBUG }} BUILD_DESTINATION=${BUILD_DESTINATION} ${SRC_DIR}/devops/build/automation/cloudberry/scripts/configure-cloudberry.sh"; then
463462
echo "::error::Configure script failed"
464463
exit 1
465464
fi

devops/build/automation/cloudberry/scripts/configure-cloudberry.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ execute_cmd ./configure --prefix=${BUILD_DESTINATION} \
163163
--disable-pxf \
164164
--enable-tap-tests \
165165
${CONFIGURE_DEBUG_OPTS} \
166+
--with-gp-stats-collector \
166167
--with-gssapi \
167168
--with-ldap \
168169
--with-libxml \

gpcontrib/gp_stats_collector/src/Config.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ extern "C" {
4040
static char *guc_uds_path = nullptr;
4141
static bool guc_enable_analyze = true;
4242
static bool guc_enable_cdbstats = true;
43-
static bool guc_enable_collector = true;
43+
static bool guc_enable_collector = false;
4444
static bool guc_report_nested_queries = true;
4545
static char *guc_ignored_users = nullptr;
4646
static int guc_max_text_size = 1 << 20; // in bytes (1MB)
@@ -68,7 +68,7 @@ void Config::init_gucs() {
6868

6969
DefineCustomBoolVariable(
7070
"gpsc.enable", "Enable metrics collector", 0LL, &guc_enable_collector,
71-
true, PGC_SUSET, GUC_NOT_IN_SAMPLE | GUC_GPDB_NEED_SYNC, 0LL, 0LL, 0LL);
71+
false, PGC_SUSET, GUC_NOT_IN_SAMPLE | GUC_GPDB_NEED_SYNC, 0LL, 0LL, 0LL);
7272

7373
DefineCustomBoolVariable(
7474
"gpsc.enable_analyze", "Collect analyze metrics in gpsc", 0LL,
@@ -88,7 +88,7 @@ void Config::init_gucs() {
8888
DefineCustomStringVariable("gpsc.ignored_users_list",
8989
"Make gpsc ignore queries issued by given users",
9090
0LL, &guc_ignored_users,
91-
"gpadmin,repl,gpperfmon,monitor", PGC_SUSET,
91+
"", PGC_SUSET,
9292
GUC_NOT_IN_SAMPLE | GUC_GPDB_NEED_SYNC, 0LL,
9393
assign_ignored_users_hook, 0LL);
9494

gpcontrib/gp_stats_collector/src/EventSender.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ bool EventSender::verify_query(QueryDesc *query_desc, QueryState state,
6868
// not executed yet, causing DONE to be skipped/added.
6969
config.sync();
7070

71+
if (!config.enable_collector()) {
72+
return false;
73+
}
74+
7175
if (utility && !config.enable_utility()) {
7276
return false;
7377
}
@@ -409,13 +413,11 @@ EventSender::EventSender() {
409413
// Perform initial sync to get default GUC values
410414
config.sync();
411415

412-
if (config.enable_collector()) {
413-
try {
414-
GOOGLE_PROTOBUF_VERIFY_VERSION;
415-
proto_verified = true;
416-
} catch (const std::exception &e) {
417-
ereport(INFO, (errmsg("Unable to start query tracing %s", e.what())));
418-
}
416+
try {
417+
GOOGLE_PROTOBUF_VERIFY_VERSION;
418+
proto_verified = true;
419+
} catch (const std::exception &e) {
420+
ereport(INFO, (errmsg("GPSC protobuf version mismatch is detected %s", e.what())));
419421
}
420422
#ifdef IC_TEARDOWN_HOOK
421423
memset(&ic_statistics, 0, sizeof(ICStatistics));

0 commit comments

Comments
 (0)