Update ci pipeline to config WITH_LOG_STATE#353
Conversation
WalkthroughThe PR adds explicit Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
concourse/scripts/build_tarball.bash (1)
202-202: Add validation for WITH_LOG_STATE parameter.The script now relies on explicit WITH_LOG_STATE from the caller (previously derived from DATA_STORE_TYPE). Without validation, an unset or empty WITH_LOG_STATE will be silently passed to CMake, potentially causing undefined build behavior.
Since pipelines now explicitly provide this parameter, consider adding a guard to catch configuration errors:
if [ -z "${WITH_LOG_STATE:-}" ]; then echo "ERROR: WITH_LOG_STATE must be provided. Supported: ROCKSDB, ROCKSDB_CLOUD_S3, ROCKSDB_CLOUD_GCS" exit 1 fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
concourse/pipeline/build_debug_tarball.yml(4 hunks)concourse/pipeline/build_nightly_tarball.yml(8 hunks)concourse/pipeline/build_release_tarball.yml(14 hunks)concourse/scripts/build_tarball.bash(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: githubzilla
Repo: eloqdata/eloqdoc PR: 211
File: src/mongo/db/modules/eloq/cmake/build_eloq_log_service.cmake:26-80
Timestamp: 2025-09-25T12:24:06.434Z
Learning: The user githubzilla implemented a fix in commit fe98aaf to address the MEMORY state incorrectly triggering RocksDB discovery in build_eloq_log_service.cmake. This was a control flow issue where MEMORY builds were falling into an else branch that still attempted RocksDB discovery.
Learnt from: githubzilla
Repo: eloqdata/eloqdoc PR: 211
File: src/mongo/db/modules/eloq/cmake/build_log_service.cmake:116-119
Timestamp: 2025-09-25T11:58:50.446Z
Learning: The build_log_service.cmake file is specifically for "open log" functionality and only supports the ROCKSDB log state type (LOG_STATE_TYPE_RKDB). The full log state configuration with cloud variants (ROCKSDB_CLOUD_S3, ROCKSDB_CLOUD_GCS) is handled in build_eloq_log_service.cmake.
📚 Learning: 2025-09-25T11:58:50.446Z
Learnt from: githubzilla
Repo: eloqdata/eloqdoc PR: 211
File: src/mongo/db/modules/eloq/cmake/build_log_service.cmake:116-119
Timestamp: 2025-09-25T11:58:50.446Z
Learning: The build_log_service.cmake file is specifically for "open log" functionality and only supports the ROCKSDB log state type (LOG_STATE_TYPE_RKDB). The full log state configuration with cloud variants (ROCKSDB_CLOUD_S3, ROCKSDB_CLOUD_GCS) is handled in build_eloq_log_service.cmake.
Applied to files:
concourse/pipeline/build_release_tarball.ymlconcourse/pipeline/build_debug_tarball.ymlconcourse/pipeline/build_nightly_tarball.ymlconcourse/scripts/build_tarball.bash
📚 Learning: 2025-09-25T12:24:06.434Z
Learnt from: githubzilla
Repo: eloqdata/eloqdoc PR: 211
File: src/mongo/db/modules/eloq/cmake/build_eloq_log_service.cmake:26-80
Timestamp: 2025-09-25T12:24:06.434Z
Learning: The user githubzilla implemented a fix in commit fe98aaf to address the MEMORY state incorrectly triggering RocksDB discovery in build_eloq_log_service.cmake. This was a control flow issue where MEMORY builds were falling into an else branch that still attempted RocksDB discovery.
Applied to files:
concourse/pipeline/build_release_tarball.ymlconcourse/pipeline/build_debug_tarball.ymlconcourse/scripts/build_tarball.bash
📚 Learning: 2025-09-25T12:09:01.276Z
Learnt from: githubzilla
Repo: eloqdata/eloqdoc PR: 211
File: src/mongo/db/modules/eloq/src/eloq_kv_engine.cpp:95-115
Timestamp: 2025-09-25T12:09:01.276Z
Learning: The log states (LOG_STATE_TYPE_RKDB, LOG_STATE_TYPE_RKDB_S3, LOG_STATE_TYPE_RKDB_GCS) in the EloqDoc codebase are mutually exclusive - only one can be active at a time. The LOG_STATE_TYPE_RKDB_CLOUD macro is a helper that should only be defined for cloud variants (S3/GCS) and not when regular RKDB is active.
Applied to files:
concourse/pipeline/build_release_tarball.ymlconcourse/pipeline/build_debug_tarball.ymlconcourse/scripts/build_tarball.bash
📚 Learning: 2025-11-28T10:55:21.020Z
Learnt from: githubzilla
Repo: eloqdata/eloqdoc PR: 279
File: src/mongo/db/modules/eloq/CMakeLists.txt:54-55
Timestamp: 2025-11-28T10:55:21.020Z
Learning: In src/mongo/db/modules/eloq/CMakeLists.txt, the CMake options EXT_TX_PROC_ENABLED, FORK_HM_PROCESS, and STATISTICS are used by the data_substrate submodule build system and should be retained as configuration interface options.
Applied to files:
concourse/pipeline/build_release_tarball.ymlconcourse/scripts/build_tarball.bash
📚 Learning: 2025-11-28T10:52:25.527Z
Learnt from: githubzilla
Repo: eloqdata/eloqdoc PR: 279
File: concourse/scripts/main.ent.bash:38-39
Timestamp: 2025-11-28T10:52:25.527Z
Learning: In the eloqdoc enterprise build (main.ent.bash), eloq_log_service is fetched separately as the resource `eloq_logservice_src` in the pipeline (main.ent.yml) and then symlinked into the data_substrate directory structure at build time using `ln -s $WORKSPACE/eloq_logservice_src eloq_log_service`. This is the intended architecture for placing eloq_log_service within data_substrate in the enterprise configuration.
Applied to files:
concourse/pipeline/build_release_tarball.ymlconcourse/pipeline/build_debug_tarball.ymlconcourse/pipeline/build_nightly_tarball.ymlconcourse/scripts/build_tarball.bash
📚 Learning: 2025-11-24T03:11:24.955Z
Learnt from: lzxddz
Repo: eloqdata/eloqdoc PR: 325
File: src/mongo/db/modules/eloq/src/eloq_kv_engine.cpp:1362-1384
Timestamp: 2025-11-24T03:11:24.955Z
Learning: The EloqDoc codebase uses an older version of glog that supports the two-parameter signature `google::InitGoogleLogging(const char* argv0, PrefixCallback)` for custom log prefix formatting, as seen in src/mongo/db/modules/eloq/src/eloq_kv_engine.cpp line 1383.
Applied to files:
concourse/scripts/build_tarball.bash
🔇 Additional comments (3)
concourse/pipeline/build_nightly_tarball.yml (1)
80-80: WITH_LOG_STATE parameters are consistent within nightly pipeline.All mappings correctly associate DATA_STORE_TYPE variants with their corresponding log states. ELOQDSS_ELOQSTORE consistently maps to ROCKSDB_CLOUD_S3 across all ubuntu versions.
Also applies to: 114-114, 146-146, 164-164, 196-196, 229-229, 246-246, 278-278
concourse/pipeline/build_release_tarball.yml (1)
88-88: Standard log state mappings are consistent in release pipeline.The ROCKSDB_CLOUD_S3 and ROCKSDB mappings are correctly applied across all OS variants (ubuntu2004, ubuntu2204, ubuntu2404, centos7, centos8, rocky9).
Also applies to: 104-104, 154-154, 170-170, 222-222, 239-239, 271-271, 287-287, 319-319, 335-335, 367-367, 383-383
concourse/pipeline/build_debug_tarball.yml (1)
67-67: WITH_LOG_STATE parameters are correctly configured in debug pipeline.All four build variants have appropriate log state mappings. Notably, ELOQDSS_ELOQSTORE maps to ROCKSDB_CLOUD_S3, consistent with the nightly pipeline.
Also applies to: 85-85, 103-103, 121-121
| BUILD_LOG_SRV: "true" | ||
| BUILD_TYPE: RelWithDebInfo | ||
| DATA_STORE_TYPE: ELOQDSS_ELOQSTORE | ||
| WITH_LOG_STATE: ROCKSDB_CLOUD_GCS |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "build_release_tarball.yml" -o -name "*debug*" -name "*.yml" | grep -i pipeline | head -20Repository: eloqdata/eloqdoc
Length of output: 151
🏁 Script executed:
fd -e yml -e yaml . concourse/pipeline/ 2>/dev/null | head -20Repository: eloqdata/eloqdoc
Length of output: 351
🏁 Script executed:
wc -l concourse/pipeline/build_release_tarball.yml concourse/pipeline/build_debug_tarball.yml concourse/pipeline/build_nightly_tarball.ymlRepository: eloqdata/eloqdoc
Length of output: 222
🏁 Script executed:
sed -n '130,150p' concourse/pipeline/build_release_tarball.ymlRepository: eloqdata/eloqdoc
Length of output: 804
🏁 Script executed:
sed -n '200,210p' concourse/pipeline/build_release_tarball.ymlRepository: eloqdata/eloqdoc
Length of output: 463
🏁 Script executed:
rg "ELOQDSS_ELOQSTORE" concourse/pipeline/ -A 2Repository: eloqdata/eloqdoc
Length of output: 1312
🏁 Script executed:
rg "WITH_LOG_STATE" concourse/pipeline/Repository: eloqdata/eloqdoc
Length of output: 2180
🏁 Script executed:
cat -n concourse/pipeline/build_debug_tarball.ymlRepository: eloqdata/eloqdoc
Length of output: 4437
🏁 Script executed:
cat -n concourse/pipeline/build_nightly_tarball.ymlRepository: eloqdata/eloqdoc
Length of output: 11462
🏁 Script executed:
sed -n '130,145p' concourse/pipeline/build_release_tarball.ymlRepository: eloqdata/eloqdoc
Length of output: 609
🏁 Script executed:
sed -n '200,215p' concourse/pipeline/build_release_tarball.ymlRepository: eloqdata/eloqdoc
Length of output: 602
Inconsistent WITH_LOG_STATE mapping for ELOQDSS_ELOQSTORE across pipelines.
The release pipeline maps ELOQDSS_ELOQSTORE to ROCKSDB_CLOUD_GCS (lines 138, 205), while the nightly and debug pipelines map it to ROCKSDB_CLOUD_S3. This discrepancy means identical input (DATA_STORE_TYPE: ELOQDSS_ELOQSTORE) produces different log state configurations, resulting in incompatible binaries across pipeline environments.
Align the mappings across all three pipelines (debug, nightly, release).
🤖 Prompt for AI Agents
In concourse/pipeline/build_release_tarball.yml around line 138 (and also update
the mapping at line 205), the WITH_LOG_STATE for DATA_STORE_TYPE
ELOQDSS_ELOQSTORE is set to ROCKSDB_CLOUD_GCS which is inconsistent with the
nightly and debug pipelines that use ROCKSDB_CLOUD_S3; change the mapping(s) in
the release pipeline to ROCKSDB_CLOUD_S3 so that ELOQDSS_ELOQSTORE yields the
same log state across debug, nightly, and release pipelines, and scan other
pipeline files for any other mismatched mappings to ensure consistency.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.