Skip to content

ci: remove EloqStore fd limit override and bump data_substrate - #483

Merged
thweetkomputer merged 6 commits into
mainfrom
agent/remove-eloqstore-fd-limit
Jul 27, 2026
Merged

ci: remove EloqStore fd limit override and bump data_substrate#483
thweetkomputer merged 6 commits into
mainfrom
agent/remove-eloqstore-fd-limit

Conversation

@thweetkomputer

@thweetkomputer thweetkomputer commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

What

  • Remove the GitHub CI generated data_substrate.cnf override for eloq_store_open_files_limit=512.
  • Advance data_substrate to 4ef1ca4f58.

Why

  • Run CI with the new data_substrate EloqStore file descriptor behavior instead of forcing the old 512 limit.

Validation

  • bash -n .github/scripts/common.sh
  • git diff --check

Summary by CodeRabbit

  • Bug Fixes

    • Removed an outdated file-limit configuration from the runtime setup to improve compatibility.
    • Improved behavior when core-dump configuration cannot be applied, with clearer warnings.
  • CI and Diagnostics

    • Expanded CI failure output with richer metadata, running-process listings, live backtraces (when possible), and symbolized core-dump backtraces.
    • Improves resilience by attempting to prepare debugging tools when available.
  • Maintenance

    • Updated the embedded data storage component to a newer revision.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The PR removes the EloqStore open-files limit from runtime configuration, adds GDB-based live and core backtrace collection to CI failure diagnostics, makes core-pattern setup tolerant of permission failures, and advances the data_substrate submodule reference.

Changes

CI diagnostics and runtime configuration

Layer / File(s) Summary
Runtime configuration updates
.github/artifact/ELOQDSS_ELOQSTORE/data_substrate.cnf, .github/scripts/common.sh
Removes eloq_store_open_files_limit from shipped and generated EloqStore configuration.
Failure backtrace collection
.github/scripts/common.sh, .github/scripts/gh_ci_entry.sh
Adds GDB availability checks, live-process backtraces, core-file symbolization, expanded failure diagnostics, and tolerant core_pattern configuration.

Substrate revision

Layer / File(s) Summary
Submodule revision update
src/mongo/db/modules/eloq/data_substrate
Advances the pinned data_substrate submodule commit.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CI as CI failure handler
  participant GDB as gdb
  participant Live as Eloq processes
  participant Core as Core files
  CI->>GDB: ensure availability
  CI->>Live: discover running processes
  GDB->>Live: collect thread backtraces
  CI->>Core: locate newest core files
  GDB->>Core: identify program and print backtraces
Loading

Possibly related PRs

Suggested labels: trigger-ci

Suggested reviewers: liunyl

Poem

A rabbit checks the failing trail,
And asks GDB to tell the tale.
Live threads hop, core files gleam,
Config lines vanish from the stream.
The substrate pin moves on—
CI finds clues before dawn.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main CI change: removing the EloqStore file-descriptor override and bumping data_substrate.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/remove-eloqstore-fd-limit

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thweetkomputer
thweetkomputer marked this pull request as ready for review July 24, 2026 08:18

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/scripts/common.sh (1)

629-648: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider pgrep over ps | grep per static analysis hint.

Static analysis flags parsing ps -ef with grep (SC2009); pgrep -a avoids the extra process and self-match filtering. Purely cosmetic here since output is just printed for diagnostics.

♻️ Proposed refactor
-  ps -ef | grep -E 'eloqdoc|dss_server|launch_sv|host_manager|minio' | grep -v grep || true
+  pgrep -a -f 'eloqdoc|dss_server|launch_sv|host_manager|minio' || true
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/scripts/common.sh around lines 629 - 648, Update
dump_ci_failure_logs to replace the ps -ef piped through grep process listing
with pgrep -a using the existing eloqdoc, dss_server, launch_sv, host_manager,
and minio patterns. Preserve the current diagnostic output and non-failing
behavior when no matching processes are found.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/scripts/common.sh:
- Around line 544-574: Update find_binary_for_core so fallback candidates are
considered only when the requested base name is not among the known candidate
names, and exclude any name already searched. Preserve exact-name lookup first,
and return the symbol-less path rather than associating a core with an unrelated
binary.

In `@src/mongo/db/modules/eloq/data_substrate`:
- Line 1: Update the data_substrate submodule gitlink to an existing, resolvable
commit SHA representing the intended dependency version, replacing the invalid
9125a4cc... revision. Keep the src/mongo/db/modules/eloq/SConscript dependency
path unchanged.

---

Nitpick comments:
In @.github/scripts/common.sh:
- Around line 629-648: Update dump_ci_failure_logs to replace the ps -ef piped
through grep process listing with pgrep -a using the existing eloqdoc,
dss_server, launch_sv, host_manager, and minio patterns. Preserve the current
diagnostic output and non-failing behavior when no matching processes are found.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d62373a1-de30-470d-8bf3-b3077e237e15

📥 Commits

Reviewing files that changed from the base of the PR and between 818fc14 and 7e0549c.

📒 Files selected for processing (4)
  • .github/artifact/ELOQDSS_ELOQSTORE/data_substrate.cnf
  • .github/scripts/common.sh
  • .github/scripts/gh_ci_entry.sh
  • src/mongo/db/modules/eloq/data_substrate
💤 Files with no reviewable changes (1)
  • .github/artifact/ELOQDSS_ELOQSTORE/data_substrate.cnf

Comment thread .github/scripts/common.sh
Comment thread src/mongo/db/modules/eloq/data_substrate
@thweetkomputer thweetkomputer changed the title ci: remove EloqStore fd limit override ci: remove EloqStore fd limit override and bump data_substrate Jul 27, 2026
@thweetkomputer
thweetkomputer merged commit 2fb243f into main Jul 27, 2026
17 checks passed
@thweetkomputer
thweetkomputer deleted the agent/remove-eloqstore-fd-limit branch July 27, 2026 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants