Skip to content

fix: set float/double precision in JSON output#4

Open
rophy wants to merge 2 commits intomasterfrom
fix/3
Open

fix: set float/double precision in JSON output#4
rophy wants to merge 2 commits intomasterfrom
fix/3

Conversation

@rophy
Copy link
Owner

@rophy rophy commented Mar 6, 2026

Summary

Fixes issue #3 (Issue B): BINARY_DOUBLE and BINARY_FLOAT values truncated to 6 significant digits in JSON output.

  • columnFloat(): set std::setprecision(9) (IEEE 754 single needs 9 digits for round-trip)
  • columnDouble(): set std::setprecision(17) (IEEE 754 double needs 17 digits for round-trip)

Before: 65536.12565536.1, 3.141592653589793.14159
After: 65536.12565536.125, 3.141592653589793.1415926535897931

Test plan

  • All 41 pipeline tests pass (golden files regenerated)
  • Debezium twin-test confirms no more COL_DOUBLE diffs

Summary by CodeRabbit

  • New Features

    • End-to-end Debezium test harness: Docker Compose environment, Oracle init scripts, test runner, HTTP receiver, and comparison tool for LogMiner vs OLR outputs.
  • Improvements

    • JSON numeric formatting clarified: explicit precision for floating-point and double values.
  • Chores

    • Added Debezium configs, Makefile, helper scripts, test fixtures, and adjusted .gitignore and prebuilt test data.

columnFloat: setprecision(9) for IEEE 754 single (was default 6)
columnDouble: setprecision(17) for IEEE 754 double (was default 6)

Fixes BINARY_DOUBLE truncation, e.g. 65536.125 was output as 65536.1.
@coderabbitai
Copy link

coderabbitai bot commented Mar 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 357a0623-474a-47d3-befa-06058f46e044

📥 Commits

Reviewing files that changed from the base of the PR and between 1b6583d and 9d5db6e.

📒 Files selected for processing (5)
  • tests/2-prebuilt/expected/basic-crud/logminer-reference.json
  • tests/2-prebuilt/expected/basic-crud/output.json
  • tests/2-prebuilt/redo/.gitignore
  • tests/2-prebuilt/schema/.gitignore
  • tests/2-prebuilt/schema/basic-crud/TEST-chkpt-3762378.json
💤 Files with no reviewable changes (4)
  • tests/2-prebuilt/redo/.gitignore
  • tests/2-prebuilt/expected/basic-crud/output.json
  • tests/2-prebuilt/expected/basic-crud/logminer-reference.json
  • tests/2-prebuilt/schema/.gitignore

📝 Walkthrough

Walkthrough

Adds Debezium twin-test infrastructure (Oracle, OLR, Debezium LogMiner, receiver, orchestration and comparison scripts) and tweaks JSON numeric formatting precision in BuilderJson.cpp (float = 9, double = 17).

Changes

Cohort / File(s) Summary
JSON Numeric Formatting
src/builder/BuilderJson.cpp
Sets explicit streaming precision for floats/doubles: columnFloat uses std::setprecision(9), columnDouble uses std::setprecision(17).
Debezium test infra — config & compose
tests/debezium/docker-compose.yaml, tests/debezium/config/application-logminer.properties, tests/debezium/config/application-olr.properties, tests/debezium/config/olr-config.json
Adds Docker Compose and Debezium/OpenLogReplicator configuration files for LogMiner and OLR end-to-end tests, including sink, source, connector and offset/schema-history settings.
Debezium test infra — init & orchestration scripts
tests/debezium/oracle-init/..., tests/scripts/run-debezium.sh
Adds Oracle initialization scripts to enable ARCHIVELOG/supplemental logging and provision c##dbzuser, plus run-debezium.sh to orchestrate staged test runs, health checks, log switches, and sentinel injection.
Debezium test infra — receiver & comparison utilities
tests/scripts/debezium-receiver.py, tests/scripts/compare-debezium.py
Adds an HTTP receiver that collects LogMiner/OLR events to separate JSONL files (with status/reset endpoints) and a comparator that normalizes and matches records with numeric tolerance to detect mismatches.
Debezium test infra — Makefile & helper files
tests/debezium/Makefile, tests/debezium/.gitignore
Adds Makefile targets for downloading JDBC driver, bringing up/down compose services, and running scenarios; updates .gitignore to ignore lib/, output/, data/.
Debezium test infra — test data adjustments
tests/2-prebuilt/..., tests/2-prebuilt/.../.gitignore
Removes certain prebuilt expected outputs and modifies .gitignore entries across prebuilt test directories (deleted expected JSON and adjusted ignore patterns).

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Orchestrator as run-debezium.sh
participant Oracle as Oracle XE
participant OLR as OpenLogReplicator
participant DebeziumLog as Debezium(LogMiner)
participant DebeziumOLR as Debezium(OLR)
participant Receiver as HTTP Receiver
participant Comparator as compare-debezium.py

Orchestrator->>Oracle: provision (01-setup.sh, 02-debezium-setup.sh)
Orchestrator->>DebeziumLog: start connector (docker compose)
Orchestrator->>DebeziumOLR: start connector (docker compose)
Orchestrator->>OLR: start OLR service
Orchestrator->>Receiver: ensure /status healthy
Orchestrator->>Oracle: apply scenario SQL (DDL/DML, log switch)
Oracle->>DebeziumLog: stream CDC events -> Receiver (HTTP)
OLR->>DebeziumOLR: stream CDC events -> Receiver (HTTP)
Receiver->>Orchestrator: report sentinel processed
Orchestrator->>Comparator: invoke comparison on collected JSONL
Comparator->>Orchestrator: return pass/fail

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰
I dug a test tunnel late at night,
Two adapters dancing in docker-light,
Sentinels hop in, events align,
Nine and seventeen digits shine,
A carrot-cheer for CDC, crisp and bright.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'fix: set float/double precision in JSON output' clearly summarizes the main change in the changeset, which is adjusting JSON numeric formatting for floating-point and double precision values.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/3

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 and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (2)
tests/scripts/compare-debezium.py (1)

67-84: Edge case in relative tolerance: asymmetric comparison may miss mismatches.

The relative tolerance check at line 80 only considers abs(fa) as the denominator. If fa is very small (e.g., 0.000001) but fb is much larger (e.g., 1.0), the condition abs(fa - fb) / abs(fa) would be very large and fail as expected. However, if the values are swapped in the comparison (due to LogMiner vs OLR ordering), the behavior differs.

More robust approach: use the larger absolute value as the reference:

♻️ Suggested fix for symmetric relative tolerance
     # Numeric tolerance
     try:
         fa, fb = float(a), float(b)
         if fa == fb:
             return True
-        if fa != 0 and abs(fa - fb) / abs(fa) < 1e-6:
+        max_abs = max(abs(fa), abs(fb))
+        if max_abs != 0 and abs(fa - fb) / max_abs < 1e-6:
             return True
     except (ValueError, TypeError):
         pass
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/scripts/compare-debezium.py` around lines 67 - 84, The
relative-tolerance check in values_match currently divides by abs(fa) which is
asymmetric; update values_match to compute fa, fb as floats then use a symmetric
denominator like max(abs(fa), abs(fb)) (and treat denominator == 0 as equal)
when evaluating abs(fa - fb) / denominator < 1e-6 so that comparisons are
order-independent; keep the existing exact-equality and None checks and still
catch ValueError/TypeError around the float conversion.
tests/scripts/run-debezium.sh (1)

120-145: Wait loops silently succeed on timeout, potentially masking initialization failures.

The loops at lines 122-127, 132-137, and 139-144 will silently continue if the expected log patterns never appear. This could lead to flaky or misleading test results when OLR or connectors fail to initialize properly.

Consider adding timeout detection similar to Stage 5's approach:

♻️ Suggested fix for OLR wait loop (apply similar pattern to connector waits)
 # Wait for OLR to start processing redo logs
 echo "  Waiting for OLR to initialize..."
+OLR_READY=false
 for i in $(seq 1 60); do
     if docker logs dbz-olr 2>&1 | tail -5 | grep -q "processing redo log"; then
+        OLR_READY=true
         break
     fi
     sleep 2
 done
+if [[ "$OLR_READY" != "true" ]]; then
+    echo "WARNING: OLR may not have initialized properly (timeout after 120s)" >&2
+fi
 echo "  OLR: ready"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/scripts/run-debezium.sh` around lines 120 - 145, The three wait-for
loops that poll docker logs for containers dbz-olr, dbz-olr-adapter, and
dbz-logminer should detect timeouts and fail loudly instead of silently
continuing; after each loop (the one checking for "processing redo log" on
dbz-olr, the one checking for "streaming client started successfully|Starting
streaming" on dbz-olr-adapter, and the one checking for "Starting streaming" on
dbz-logminer) add an if that re-checks the grep condition and, if not matched,
echo an error with context, dump the recent docker logs for that container, and
exit 1 so the script fails fast on initialization errors. Ensure you reference
the exact container names (dbz-olr, dbz-olr-adapter, dbz-logminer) and log
patterns when implementing the timeout checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/debezium/docker-compose.yaml`:
- Around line 32-47: Add a healthcheck to the olr service that verifies
OpenLogReplicator is listening on port 5000 (e.g., a TCP or HTTP check against
port 5000) and then gate the Debezium OLR adapter container (the service using
container_name dbz-olr / service name that runs the adapter) on olr’s readiness
by adding a depends_on entry pointing to olr with condition: service_healthy;
update the olr service’s healthcheck parameters (test, interval, timeout,
retries) so the compose wait is robust and Debezium won’t attempt to connect
before port 5000 is ready.
- Around line 20-31: The Debezium services currently only wait for the receiver
container to start, not to be ready, causing race conditions; add a Docker
healthcheck to the receiver service (the service named "receiver" running
command ["python3", "/app/debezium-receiver.py"] and exposing port 8080) that
probes the HTTP endpoint (e.g., curl http://localhost:8080/ or /health with a
short timeout and retries) and then update the Debezium services' depends_on
entries to require service_healthy for "receiver" so compose --wait will wait
for the receiver's healthcheck to pass before starting the Debezium services.

In `@tests/debezium/Makefile`:
- Around line 10-14: The download-ojdbc target currently treats any existing
lib/ojdbc8.jar as valid and can leave a zero-byte/truncated file; change the
recipe to download to a temporary file (e.g., lib/ojdbc8.jar.tmp or using
mktemp), verify wget succeeded, then atomically mv the temp file to
lib/ojdbc8.jar; ensure the lib directory is created first, remove the temp file
on failure, and still skip download when a valid lib/ojdbc8.jar exists;
reference the download-ojdbc target and the OJDBC_VERSION/OJDBC_URL variables
and the lib/ojdbc8.jar destination when making these changes.

In `@tests/debezium/oracle-init/01-setup.sh`:
- Around line 5-15: The SQL*Plus block lacks error handling and sets ALTER
SYSTEM SET log_archive_dest_1 before the target directory exists; add WHENEVER
SQLERROR EXIT SQL.SQLCODE and WHENEVER OSERROR EXIT FAILURE at the top of the
sqlplus session to abort on errors, move the HOST mkdir -p
/opt/oracle/oradata/XE/archive so it runs before ALTER SYSTEM SET
log_archive_dest_1='LOCATION=...' (so the path exists when Oracle validates it),
and ensure the sqlplus invocation checks the exit code so the init script fails
fast if either the mkdir or ALTER SYSTEM call fails.

In `@tests/debezium/oracle-init/02-debezium-setup.sh`:
- Line 5: Add SQL*Plus error handling at the start of the heredoc launched by
"sqlplus -S / as sysdba <<'SQL'" so the script stops on the first SQL or OS
error; insert the two directives ("WHENEVER SQLERROR ..." and "WHENEVER OSERROR
...") immediately after the heredoc begins to force an immediate exit on
failures (so bad GRANTs for c##dbzuser halt provisioning).

---

Nitpick comments:
In `@tests/scripts/compare-debezium.py`:
- Around line 67-84: The relative-tolerance check in values_match currently
divides by abs(fa) which is asymmetric; update values_match to compute fa, fb as
floats then use a symmetric denominator like max(abs(fa), abs(fb)) (and treat
denominator == 0 as equal) when evaluating abs(fa - fb) / denominator < 1e-6 so
that comparisons are order-independent; keep the existing exact-equality and
None checks and still catch ValueError/TypeError around the float conversion.

In `@tests/scripts/run-debezium.sh`:
- Around line 120-145: The three wait-for loops that poll docker logs for
containers dbz-olr, dbz-olr-adapter, and dbz-logminer should detect timeouts and
fail loudly instead of silently continuing; after each loop (the one checking
for "processing redo log" on dbz-olr, the one checking for "streaming client
started successfully|Starting streaming" on dbz-olr-adapter, and the one
checking for "Starting streaming" on dbz-logminer) add an if that re-checks the
grep condition and, if not matched, echo an error with context, dump the recent
docker logs for that container, and exit 1 so the script fails fast on
initialization errors. Ensure you reference the exact container names (dbz-olr,
dbz-olr-adapter, dbz-logminer) and log patterns when implementing the timeout
checks.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4a53acd2-80ca-4698-8383-da35271a4398

📥 Commits

Reviewing files that changed from the base of the PR and between 0d8b3b2 and 1b6583d.

📒 Files selected for processing (12)
  • src/builder/BuilderJson.cpp
  • tests/debezium/.gitignore
  • tests/debezium/Makefile
  • tests/debezium/config/application-logminer.properties
  • tests/debezium/config/application-olr.properties
  • tests/debezium/config/olr-config.json
  • tests/debezium/docker-compose.yaml
  • tests/debezium/oracle-init/01-setup.sh
  • tests/debezium/oracle-init/02-debezium-setup.sh
  • tests/scripts/compare-debezium.py
  • tests/scripts/debezium-receiver.py
  • tests/scripts/run-debezium.sh

Comment on lines +20 to +31
receiver:
image: python:3.12-slim
container_name: dbz-receiver
command: ["python3", "/app/debezium-receiver.py"]
ports:
- "8080:8080"
environment:
OUTPUT_DIR: /app/output
volumes:
- ../scripts/debezium-receiver.py:/app/debezium-receiver.py:ro
- ./output:/app/output

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# First, let's get the full docker-compose.yaml file
echo "=== Full docker-compose.yaml file ==="
cat -n tests/debezium/docker-compose.yaml

Repository: rophy/OpenLogReplicator

Length of output: 2896


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Checking Makefile for docker compose commands ==="
cat -n tests/debezium/Makefile | grep -A 2 -B 2 "docker compose"

echo
echo "=== Checking debezium-receiver.py for port 8080 setup ==="
if [ -f scripts/debezium-receiver.py ]; then
  head -100 scripts/debezium-receiver.py | grep -A 5 -B 5 "8080\|listen\|port" || echo "Pattern not found in first 100 lines"
fi

Repository: rophy/OpenLogReplicator

Length of output: 331


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Full debezium-receiver.py file ==="
cat -n scripts/debezium-receiver.py

Repository: rophy/OpenLogReplicator

Length of output: 183


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Searching for debezium-receiver.py ==="
fd -t f "debezium-receiver.py"

echo
echo "=== Searching for receiver scripts ==="
fd -t f -i "receiver" | head -20

Repository: rophy/OpenLogReplicator

Length of output: 219


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Full debezium-receiver.py file ==="
cat -n tests/scripts/debezium-receiver.py

Repository: rophy/OpenLogReplicator

Length of output: 6380


Gate the Debezium services on receiver readiness, not just startup.

make up uses docker compose up --wait, but receiver has no healthcheck and both Debezium services only depend on it being "started". If the Python HTTP server has not bound :8080 yet, the first posts can race and make the twin-test flaky.

🛠️ Suggested fix
   receiver:
     image: python:3.12-slim
     container_name: dbz-receiver
     command: ["python3", "/app/debezium-receiver.py"]
     ports:
       - "8080:8080"
     environment:
       OUTPUT_DIR: /app/output
     volumes:
       - ../scripts/debezium-receiver.py:/app/debezium-receiver.py:ro
       - ./output:/app/output
+    healthcheck:
+      test: ["CMD", "python3", "-c", "import socket; socket.create_connection(('127.0.0.1', 8080), 2).close()"]
+      interval: 5s
+      timeout: 3s
+      retries: 20
 
   dbz-logminer:
     image: quay.io/debezium/server:3.0
     container_name: dbz-logminer
     depends_on:
       oracle:
         condition: service_healthy
       receiver:
-        condition: service_started
+        condition: service_healthy
@@
   dbz-olr:
     image: quay.io/debezium/server:3.0
     container_name: dbz-olr-adapter
     depends_on:
       olr:
         condition: service_started
       receiver:
-        condition: service_started
+        condition: service_healthy
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
receiver:
image: python:3.12-slim
container_name: dbz-receiver
command: ["python3", "/app/debezium-receiver.py"]
ports:
- "8080:8080"
environment:
OUTPUT_DIR: /app/output
volumes:
- ../scripts/debezium-receiver.py:/app/debezium-receiver.py:ro
- ./output:/app/output
receiver:
image: python:3.12-slim
container_name: dbz-receiver
command: ["python3", "/app/debezium-receiver.py"]
ports:
- "8080:8080"
environment:
OUTPUT_DIR: /app/output
volumes:
- ../scripts/debezium-receiver.py:/app/debezium-receiver.py:ro
- ./output:/app/output
healthcheck:
test: ["CMD", "python3", "-c", "import socket; socket.create_connection(('127.0.0.1', 8080), 2).close()"]
interval: 5s
timeout: 3s
retries: 20
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/debezium/docker-compose.yaml` around lines 20 - 31, The Debezium
services currently only wait for the receiver container to start, not to be
ready, causing race conditions; add a Docker healthcheck to the receiver service
(the service named "receiver" running command ["python3",
"/app/debezium-receiver.py"] and exposing port 8080) that probes the HTTP
endpoint (e.g., curl http://localhost:8080/ or /health with a short timeout and
retries) and then update the Debezium services' depends_on entries to require
service_healthy for "receiver" so compose --wait will wait for the receiver's
healthcheck to pass before starting the Debezium services.

Comment on lines +32 to +47
olr:
image: olr-dev:${OLR_IMAGE_TAG:-latest}
container_name: dbz-olr
entrypoint: ["/bin/bash", "-c", "mkdir -p /olr-data/checkpoint && exec /opt/OpenLogReplicator/OpenLogReplicator \"$@\"", "--"]
command: ["-r", "-f", "/config/olr-config.json"]
working_dir: /olr-data
group_add:
- "54321" # oinstall group — read access to Oracle redo logs
depends_on:
oracle:
condition: service_healthy
tmpfs:
- /olr-data:uid=1000,gid=54322
volumes:
- ./config/olr-config.json:/config/olr-config.json:ro
- oracle-data:/opt/oracle/oradata:ro
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if file exists and show relevant lines
file="tests/debezium/docker-compose.yaml"

if [ -f "$file" ]; then
  echo "=== File exists ==="
  echo
  
  echo "=== Lines 32-47 (olr service) ==="
  head -47 "$file" | tail -16 | cat -n
  echo
  
  echo "=== Lines 65-70 (dbz-olr and its depends_on) ==="
  wc_result=$(wc -l < "$file")
  echo "File has $wc_result total lines"
  
  if [ "$wc_result" -ge 70 ]; then
    head -70 "$file" | tail -10 | cat -n
  else
    echo "File is shorter than expected"
    tail -20 "$file" | cat -n
  fi
else
  echo "File not found: $file"
  exit 1
fi

Repository: rophy/OpenLogReplicator

Length of output: 1192


🏁 Script executed:

#!/bin/bash
# List all files in the tests directory to understand structure
if [ -d "tests" ]; then
  echo "=== Contents of tests directory ==="
  ls -la tests/ | head -20
  
  echo
  if [ -d "tests/debezium" ]; then
    echo "=== Contents of tests/debezium directory ==="
    ls -la tests/debezium/ | head -20
  fi
else
  echo "tests directory not found"
fi

Repository: rophy/OpenLogReplicator

Length of output: 1183


🏁 Script executed:

#!/bin/bash
# Search for port 5000 and OpenLogReplicator port references
echo "=== Searching for port 5000 references ==="
rg "5000" tests/debezium/

echo
echo "=== Checking OLR config file ==="
if [ -f "tests/debezium/config/olr-config.json" ]; then
  cat tests/debezium/config/olr-config.json | head -50
fi

echo
echo "=== Searching for any healthcheck patterns in the repo ==="
rg "healthcheck" . --type yaml | head -10

Repository: rophy/OpenLogReplicator

Length of output: 1628


🏁 Script executed:

#!/bin/bash
# Search for OpenLogReplicator listener or networking configuration
echo "=== Searching for listener config in codebase ==="
rg -i "listener|port.*5000|5000.*port" . --type json --type yaml | head -20

Repository: rophy/OpenLogReplicator

Length of output: 50380


Add healthcheck to olr service and gate dbz-olr on OLR readiness.

The current setup waits only for the OLR container to start, not for OpenLogReplicator to bind port 5000. This creates a race condition where Debezium's OLR adapter may attempt to connect before the service is listening.

Port 5000 is confirmed as the OLR listener port in both the OLR config (uri: 0.0.0.0:5000) and Debezium properties (openlogreplicator.port=5000).

Suggested fix
   olr:
     image: olr-dev:${OLR_IMAGE_TAG:-latest}
     container_name: dbz-olr
     entrypoint: ["/bin/bash", "-c", "mkdir -p /olr-data/checkpoint && exec /opt/OpenLogReplicator/OpenLogReplicator \"$@\"", "--"]
     command: ["-r", "-f", "/config/olr-config.json"]
     working_dir: /olr-data
     group_add:
       - "54321"  # oinstall group — read access to Oracle redo logs
     depends_on:
       oracle:
         condition: service_healthy
     tmpfs:
       - /olr-data:uid=1000,gid=54322
     volumes:
       - ./config/olr-config.json:/config/olr-config.json:ro
       - oracle-data:/opt/oracle/oradata:ro
+    healthcheck:
+      test: ["CMD", "bash", "-lc", "exec 3<>/dev/tcp/127.0.0.1/5000"]
+      interval: 5s
+      timeout: 3s
+      retries: 20
 
   dbz-olr:
     image: quay.io/debezium/server:3.0
     container_name: dbz-olr-adapter
     depends_on:
       olr:
-        condition: service_started
+        condition: service_healthy
       receiver:
         condition: service_started
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/debezium/docker-compose.yaml` around lines 32 - 47, Add a healthcheck
to the olr service that verifies OpenLogReplicator is listening on port 5000
(e.g., a TCP or HTTP check against port 5000) and then gate the Debezium OLR
adapter container (the service using container_name dbz-olr / service name that
runs the adapter) on olr’s readiness by adding a depends_on entry pointing to
olr with condition: service_healthy; update the olr service’s healthcheck
parameters (test, interval, timeout, retries) so the compose wait is robust and
Debezium won’t attempt to connect before port 5000 is ready.

Comment on lines +10 to +14
download-ojdbc:
@mkdir -p lib
@test -f lib/ojdbc8.jar || \
(echo "Downloading ojdbc8-$(OJDBC_VERSION).jar..." && \
wget -q -O lib/ojdbc8.jar $(OJDBC_URL))
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="tests/debezium/Makefile"

# First, check if the file exists
if [ ! -f "$file" ]; then
  echo "File not found: $file"
  exit 1
fi

echo "=== Full Makefile content ==="
cat -n "$file"

Repository: rophy/OpenLogReplicator

Length of output: 1140


Download the JDBC jar atomically.

This recipe treats any existing lib/ojdbc8.jar as valid. A zero-byte or truncated file from an interrupted download will be cached and every later make up will silently reuse it.

🛠️ Suggested fix
 download-ojdbc:
 	`@mkdir` -p lib
-	`@test` -f lib/ojdbc8.jar || \
-		(echo "Downloading ojdbc8-$(OJDBC_VERSION).jar..." && \
-		 wget -q -O lib/ojdbc8.jar $(OJDBC_URL))
+	`@test` -s lib/ojdbc8.jar || \
+		(tmp=$$(mktemp lib/ojdbc8.jar.XXXXXX) && \
+		 trap 'rm -f "$$tmp"' EXIT && \
+		 echo "Downloading ojdbc8-$(OJDBC_VERSION).jar..." && \
+		 wget -q -O "$$tmp" $(OJDBC_URL) && \
+		 mv "$$tmp" lib/ojdbc8.jar)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
download-ojdbc:
@mkdir -p lib
@test -f lib/ojdbc8.jar || \
(echo "Downloading ojdbc8-$(OJDBC_VERSION).jar..." && \
wget -q -O lib/ojdbc8.jar $(OJDBC_URL))
download-ojdbc:
`@mkdir` -p lib
`@test` -s lib/ojdbc8.jar || \
(tmp=$$(mktemp lib/ojdbc8.jar.XXXXXX) && \
trap 'rm -f "$$tmp"' EXIT && \
echo "Downloading ojdbc8-$(OJDBC_VERSION).jar..." && \
wget -q -O "$$tmp" $(OJDBC_URL) && \
mv "$$tmp" lib/ojdbc8.jar)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/debezium/Makefile` around lines 10 - 14, The download-ojdbc target
currently treats any existing lib/ojdbc8.jar as valid and can leave a
zero-byte/truncated file; change the recipe to download to a temporary file
(e.g., lib/ojdbc8.jar.tmp or using mktemp), verify wget succeeded, then
atomically mv the temp file to lib/ojdbc8.jar; ensure the lib directory is
created first, remove the temp file on failure, and still skip download when a
valid lib/ojdbc8.jar exists; reference the download-ojdbc target and the
OJDBC_VERSION/OJDBC_URL variables and the lib/ojdbc8.jar destination when making
these changes.

Comment on lines +5 to +15
sqlplus -S / as sysdba <<'SQL'
SHUTDOWN IMMEDIATE;
STARTUP MOUNT;
ALTER DATABASE ARCHIVELOG;
ALTER DATABASE OPEN;
ALTER DATABASE ADD SUPPLEMENTAL LOG DATA;
ALTER SYSTEM SET db_recovery_file_dest_size=10G;

-- Archive to shared volume so OLR container can read them
ALTER SYSTEM SET log_archive_dest_1='LOCATION=/opt/oracle/oradata/XE/archive' SCOPE=BOTH;
HOST mkdir -p /opt/oracle/oradata/XE/archive
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if file exists and read lines 5-15
file="tests/debezium/oracle-init/01-setup.sh"

if [ ! -f "$file" ]; then
  echo "File not found: $file"
  exit 1
fi

echo "=== Full file content ==="
cat -n "$file"
echo
echo "=== Lines 5-15 specifically ==="
sed -n '5,15p' "$file" | cat -n
echo
echo "=== Checking for WHENEVER directives ==="
grep -n 'WHENEVER' "$file" || echo "No WHENEVER directives found"
echo
echo "=== Checking order of mkdir and ALTER SYSTEM SET ==="
grep -n 'mkdir\|ALTER SYSTEM SET log_archive_dest_1' "$file"

Repository: rophy/OpenLogReplicator

Length of output: 1708


Add error handling and reorder the archive destination setup.

The script sets log_archive_dest_1 before the /opt/oracle/oradata/XE/archive directory exists, and lacks WHENEVER SQLERROR or WHENEVER OSERROR directives. If Oracle rejects the path, SQL*Plus continues silently and the init appears successful while archive logging is misconfigured.

🛠️ Suggested fix
-sqlplus -S / as sysdba <<'SQL'
+sqlplus -S / as sysdba <<'SQL'
+WHENEVER OSERROR EXIT FAILURE
+WHENEVER SQLERROR EXIT SQL.SQLCODE
 SHUTDOWN IMMEDIATE;
 STARTUP MOUNT;
 ALTER DATABASE ARCHIVELOG;
 ALTER DATABASE OPEN;
 ALTER DATABASE ADD SUPPLEMENTAL LOG DATA;
 ALTER SYSTEM SET db_recovery_file_dest_size=10G;
 
 -- Archive to shared volume so OLR container can read them
-ALTER SYSTEM SET log_archive_dest_1='LOCATION=/opt/oracle/oradata/XE/archive' SCOPE=BOTH;
 HOST mkdir -p /opt/oracle/oradata/XE/archive
+ALTER SYSTEM SET log_archive_dest_1='LOCATION=/opt/oracle/oradata/XE/archive' SCOPE=BOTH;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sqlplus -S / as sysdba <<'SQL'
SHUTDOWN IMMEDIATE;
STARTUP MOUNT;
ALTER DATABASE ARCHIVELOG;
ALTER DATABASE OPEN;
ALTER DATABASE ADD SUPPLEMENTAL LOG DATA;
ALTER SYSTEM SET db_recovery_file_dest_size=10G;
-- Archive to shared volume so OLR container can read them
ALTER SYSTEM SET log_archive_dest_1='LOCATION=/opt/oracle/oradata/XE/archive' SCOPE=BOTH;
HOST mkdir -p /opt/oracle/oradata/XE/archive
sqlplus -S / as sysdba <<'SQL'
WHENEVER OSERROR EXIT FAILURE
WHENEVER SQLERROR EXIT SQL.SQLCODE
SHUTDOWN IMMEDIATE;
STARTUP MOUNT;
ALTER DATABASE ARCHIVELOG;
ALTER DATABASE OPEN;
ALTER DATABASE ADD SUPPLEMENTAL LOG DATA;
ALTER SYSTEM SET db_recovery_file_dest_size=10G;
-- Archive to shared volume so OLR container can read them
HOST mkdir -p /opt/oracle/oradata/XE/archive
ALTER SYSTEM SET log_archive_dest_1='LOCATION=/opt/oracle/oradata/XE/archive' SCOPE=BOTH;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/debezium/oracle-init/01-setup.sh` around lines 5 - 15, The SQL*Plus
block lacks error handling and sets ALTER SYSTEM SET log_archive_dest_1 before
the target directory exists; add WHENEVER SQLERROR EXIT SQL.SQLCODE and WHENEVER
OSERROR EXIT FAILURE at the top of the sqlplus session to abort on errors, move
the HOST mkdir -p /opt/oracle/oradata/XE/archive so it runs before ALTER SYSTEM
SET log_archive_dest_1='LOCATION=...' (so the path exists when Oracle validates
it), and ensure the sqlplus invocation checks the exit code so the init script
fails fast if either the mkdir or ALTER SYSTEM call fails.

# Create Debezium user with LogMiner privileges + sentinel table.
# Runs after 01-setup.sh during gvenzl/oracle-xe container initialization.

sqlplus -S / as sysdba <<'SQL'
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if the file exists
file="tests/debezium/oracle-init/02-debezium-setup.sh"
if [ ! -f "$file" ]; then
  echo "File not found: $file"
  exit 1
fi

echo "=== Checking for WHENEVER directives ==="
rg -n 'WHENEVER (SQLERROR|OSERROR)' "$file" || echo "No WHENEVER directives found"

echo
echo "=== SQL*Plus heredoc start (lines 1-20) ==="
head -20 "$file" | cat -n

echo
echo "=== Looking for sqlplus heredoc patterns ==="
rg -n "sqlplus" "$file"

echo
echo "=== File size ==="
wc -l "$file"

Repository: rophy/OpenLogReplicator

Length of output: 1141


🏁 Script executed:

#!/bin/bash

# Check config files for c##dbzuser reference
echo "=== Checking config files for c##dbzuser ==="
find tests/debezium/config -name "application-*.properties" -type f | while read f; do
  echo "File: $f"
  rg 'c##dbzuser|dbzuser' "$f" || echo "  No dbzuser reference found"
done

Repository: rophy/OpenLogReplicator

Length of output: 308


🏁 Script executed:

#!/bin/bash

# Get the complete SQL*Plus heredoc to understand the full scope
file="tests/debezium/oracle-init/02-debezium-setup.sh"

echo "=== Complete SQL*Plus heredoc ==="
sed -n '5,/^SQL$/p' "$file" | cat -n

echo
echo "=== Context: end of file (last 10 lines) ==="
tail -10 "$file" | cat -n

Repository: rophy/OpenLogReplicator

Length of output: 4764


Stop this provisioning script on the first failed grant.

Both Debezium configs connect as c##dbzuser, so letting SQL*Plus continue past a bad grant here turns into much later connector failures. Add WHENEVER SQLERROR/OSERROR at the top of the heredoc.

🛠️ Suggested fix
-sqlplus -S / as sysdba <<'SQL'
+sqlplus -S / as sysdba <<'SQL'
+WHENEVER OSERROR EXIT FAILURE
+WHENEVER SQLERROR EXIT SQL.SQLCODE
 -- Create common user for Debezium LogMiner access (CDB level)
 CREATE USER c##dbzuser IDENTIFIED BY dbz
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sqlplus -S / as sysdba <<'SQL'
sqlplus -S / as sysdba <<'SQL'
WHENEVER OSERROR EXIT FAILURE
WHENEVER SQLERROR EXIT SQL.SQLCODE
-- Create common user for Debezium LogMiner access (CDB level)
CREATE USER c##dbzuser IDENTIFIED BY dbz
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/debezium/oracle-init/02-debezium-setup.sh` at line 5, Add SQL*Plus
error handling at the start of the heredoc launched by "sqlplus -S / as sysdba
<<'SQL'" so the script stops on the first SQL or OS error; insert the two
directives ("WHENEVER SQLERROR ..." and "WHENEVER OSERROR ...") immediately
after the heredoc begins to force an immediate exit on failures (so bad GRANTs
for c##dbzuser halt provisioning).

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.

1 participant