feat: Enable optimization by default, add logs - BED-9170 - #3105
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe change adds structured logging for graph storage optimization and moves its default configuration and status column into a separate database migration. ChangesGraph storage optimization
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
coderabbitai
Bot
added
dbmigration
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@cmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sql`:
- Line 18: Restore the original Up migration in
cmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sql
and keep its prior seed unchanged; add its Down migration to remove the
parameter and column. In
cmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql,
update only the existing prior seed to the enabled value when it still matches,
preserving externally changed values and updated_at, and add a Down migration
that restores the prior seed.
🪄 Autofix
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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 0a69a2c5-be66-4d09-814b-ae845845cd5e
📒 Files selected for processing (3)
cmd/api/src/daemons/datapipe/pipeline.gocmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sqlcmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql
|
|
||
| ALTER TABLE datapipe_status | ||
| ADD COLUMN IF NOT EXISTS last_complete_optimize_at timestamp with time zone; | ||
| -- Intentionally empty - see 20260805120000_v9_default_enable_graph_storage_optimization.sql for the up migration |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- migration files ---'
for f in \
cmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sql \
cmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql
do
echo "--- $f"
cat -n "$f"
done
printf '%s\n' '--- migration status/history ---'
git log --oneline --all -- \
cmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sql \
cmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql | head -30
printf '%s\n' '--- migration references and parameter schema/usages ---'
rg -n -C 3 \
'analysis\.graph_storage_optimization|20260722120000_v9_add_graph_storage_optimization_parameter|20260805120000_v9_default_enable_graph_storage_optimization|CREATE TABLE[[:space:]]+parameters|ON CONFLICT' \
cmd/api/src/database cmd/api/src --glob '*.sql' --glob '*.go' --glob '*.yaml' --glob '*.yml' | head -300Repository: SpecterOps/BloodHound
Length of output: 31849
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- relevant migration directory listing ---'
git ls-files cmd/api/src/database/migration/migrations | tail -40
printf '%s\n' '--- current branch and tracked-file status ---'
git status --short
git branch --show-current
printf '%s\n' '--- migration annotations and license headers ---'
for f in \
cmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sql \
cmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql
do
echo "--- $f"
sed -n '1,40p' "$f"
doneRepository: SpecterOps/BloodHound
Length of output: 5711
🏁 Script executed:
#!/bin/bash
set -e
for f in \
cmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sql \
cmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql
do
echo "--- $f ---"
cat -n "$f"
done
echo '--- commits touching these paths ---'
git log --oneline --all --follow -- cmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sql | head -20
git log --oneline --all --follow -- cmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql | head -20
echo '--- all references ---'
rg -n -C 4 'analysis\.graph_storage_optimization|20260722120000_v9_add_graph_storage_optimization_parameter|20260805120000_v9_default_enable_graph_storage_optimization|CREATE TABLE[[:space:]]+parameters|INSERT INTO parameters' . --glob '*.sql' --glob '*.go' --glob '*.yaml' --glob '*.yml' | head -400Repository: SpecterOps/BloodHound
Length of output: 33576
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
files = [
Path("cmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sql"),
Path("cmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql"),
]
for p in files:
print(f"--- {p} ---")
text = p.read_text()
print(text)
print("annotations:", [line for line in text.splitlines() if "goose" in line.lower()])
print("license first line:", text.splitlines()[0] if text.splitlines() else "")
PY
echo '--- migration filenames around v9 ---'
git ls-files 'cmd/api/src/database/migration/migrations/*.sql' | grep -E 'v9|20260722120000|20260805120000' | tail -40
echo '--- branch/ref evidence ---'
git branch -a --contains HEAD
git log --oneline --all --decorate -- cmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sql | head -20Repository: SpecterOps/BloodHound
Length of output: 5256
🏁 Script executed:
#!/bin/bash
set -e
old=cmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sql
new=cmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql
echo '--- original migration at the preceding feature commit ---'
git show 34f7b45f6e:"$old" | cat -n
echo '--- changes in the enable-by-default commit ---'
git diff --no-ext-diff --unified=8 34f7b45f6e 676a09ee2b -- "$old" "$new"
echo '--- commit metadata ---'
git show -s --format='format:%H%n%P%n%ad%n%s' --date=iso-strict 34f7b45f6e 676a09ee2bRepository: SpecterOps/BloodHound
Length of output: 5826
🏁 Script executed:
#!/bin/bash
set -e
echo '--- parameter table definition ---'
rg -n -C 10 'CREATE TABLE[[:space:]]+parameters|parameters[[:space:]]*\(' \
cmd/api/src/database/migration/migrations/00000000000001_init.sql \
cmd/api/src/database/migration/migrations/legacy/schema.sql | head -160
echo '--- parameter write paths ---'
rg -n -C 6 'UPDATE[[:space:]]+parameters|updated_at.*parameters|parameters.*updated_at|ParameterKey|GraphStorageOptimizationKey' \
cmd/api/src --glob '*.go' | head -300
echo '--- migration runner rollback behavior ---'
rg -n -C 8 'Rollback|Down|migrations|goose' cmd/api/src/database/migration/goose.go | head -260Repository: SpecterOps/BloodHound
Length of output: 50377
Preserve migration history and rollback ownership.
- Restore the original
Upand keep20260722120000_v9_add_graph_storage_optimization_parameter.sqlimmutable. Its prior seed is{"after_boot": false, "after_analysis": false, "min_interval_seconds": 86400}. - Make
20260805120000_v9_default_enable_graph_storage_optimization.sqlupdate only that prior seed to the enabled value. Do not overwrite values changed outside the migration or updateupdated_at. - Add a
Downmigration that restores the prior seed. The current emptyDownleaves the parameter and column after rollback.
📍 Affects 2 files
cmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sql#L18-L18(this comment)cmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql#L18-L27
🤖 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
`@cmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sql`
at line 18, Restore the original Up migration in
cmd/api/src/database/migration/migrations/20260722120000_v9_add_graph_storage_optimization_parameter.sql
and keep its prior seed unchanged; add its Down migration to remove the
parameter and column. In
cmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql,
update only the existing prior seed to the enabled value when it still matches,
preserving externally changed values and updated_at, and add a Down migration
that restores the prior seed.
Sources: Coding guidelines, Learnings
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@cmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql`:
- Around line 33-36: Update the migration’s parameter handling so Down removes
only the row owned and inserted by this migration, while preserving pre-existing
or user-edited values. Track ownership during Up alongside the ON CONFLICT DO
NOTHING behavior, and use that ownership marker when replacing the unconditional
DELETE for analysis.graph_storage_optimization.
🪄 Autofix
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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 5fc8a4cc-8c76-49dd-a4dd-8c5a7648a0bc
📒 Files selected for processing (1)
cmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql
Comment on lines
+33
to
+36
| ALTER TABLE datapipe_status | ||
| DROP COLUMN IF EXISTS last_complete_optimize_at; | ||
|
|
||
| DELETE FROM parameters WHERE key = 'analysis.graph_storage_optimization'; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Preserve parameter rows that this migration does not own.
ON CONFLICT DO NOTHING in Lines 18-27 allows Up to succeed without inserting the row when analysis.graph_storage_optimization already exists. Line 36 then deletes that existing row during Down. This can erase a user-edited value or a row created by an earlier migration. Track migration ownership and preserve changed rows before deleting the parameter.
🧰 Tools
🪛 Squawk (2.61.0)
[warning] 34-34: Dropping a column may break existing clients.
(ban-drop-column)
🤖 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
`@cmd/api/src/database/migration/migrations/20260805120000_v9_default_enable_graph_storage_optimization.sql`
around lines 33 - 36, Update the migration’s parameter handling so Down removes
only the row owned and inserted by this migration, while preserving pre-existing
or user-edited values. Track ownership during Up alongside the ON CONFLICT DO
NOTHING behavior, and use that ownership marker when replacing the unconditional
DELETE for analysis.graph_storage_optimization.
brandonshearin
approved these changes
Aug 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Default-enables optimization and adds logging to optimize calls.
Motivation and Context
Resolves BED-9170
Why is this change required? What problem does it solve?
How Has This Been Tested?
Validated in local environment.
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Improvements