Install update#56
Open
dvalinrh wants to merge 10 commits into
Open
Conversation
Review Summary by QodoUpdate HammerDB installation and add comprehensive TPCC benchmark support for multiple databases
WalkthroughsDescription• Refactored HammerDB installation process to download from GitHub releases instead of requiring local kit uploads • Fixed MSSQL installation to use RHEL 9 packages with dnf instead of yum • Updated all hardcoded /usr/local/HammerDB paths to use configurable $hmd_dir variable for flexible deployment • Removed upload parameter (-u) from installation scripts, simplifying the deployment workflow • Added comprehensive TPCC benchmark scripts for MariaDB, PostgreSQL, MSSQL, and Oracle databases • Implemented database build scripts with automatic configuration (buffer pools, tempdb optimization, traceflags) • Added multi-host and multi-VM benchmark execution scripts for distributed testing across multiple database instances • Created performance tuning configuration files for all supported databases (my.cnf, postgresql.conf, mssql.conf) • Added extensive documentation for TPCC setup on RHEL 7/8, OpenShift, Podman containers, and persistent memory configurations • Implemented statistics collection during benchmark runs (vmstat, iostat, system metrics) • Added Oracle database support with initialization parameters, listener configuration, and startup/shutdown scripts • Enhanced wrapper scripts to support local directory installations instead of system-wide paths Diagramflowchart LR
A["Installation Script"] -->|Download from GitHub| B["HammerDB Binary"]
A -->|Configure| C["MSSQL/MariaDB/PostgreSQL/Oracle"]
B -->|Build Schema| D["TPCC Database"]
D -->|Execute| E["Benchmark Tests"]
E -->|Collect| F["Performance Metrics"]
F -->|Generate| G["CSV Results"]
H["Multi-Host Scripts"] -->|Distribute| I["Multiple DB Instances"]
I -->|Parallel Execution| E
File Changes1. hammerdb/hammerdb_scripts/mssql/podman/update_temp.sh
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Contributor
Author
|
Qodo: Unverified root installer download, this has always been there, not worried about it. vim file removed. |
frival
reviewed
May 26, 2026
frival
reviewed
May 26, 2026
frival
reviewed
May 26, 2026
frival
reviewed
May 26, 2026
frival
reviewed
May 26, 2026
frival
reviewed
May 26, 2026
frival
reviewed
May 26, 2026
frival
reviewed
May 26, 2026
frival
reviewed
May 26, 2026
frival
reviewed
May 26, 2026
frival
reviewed
May 26, 2026
frival
reviewed
May 28, 2026
frival
reviewed
May 28, 2026
frival
reviewed
May 28, 2026
frival
reviewed
May 28, 2026
frival
reviewed
May 28, 2026
frival
reviewed
May 28, 2026
frival
reviewed
May 28, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
frival
reviewed
Jun 10, 2026
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughRefactors HammerDB installation to use a script-local ChangesHammerDB wrapper install and workload flows
🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
♻️ Duplicate comments (6)
hammerdb/hammerdb_scripts/mariadb/my.cnf (1)
4-4:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHardcoded datadir may not exist on target systems.
The datadir
/perf1/mysql/datais hardcoded and may not exist on all benchmark target systems, causing MariaDB startup failures.Consider documenting this path requirement in setup instructions or making it configurable via the build script.
🤖 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 `@hammerdb/hammerdb_scripts/mariadb/my.cnf` at line 4, The my.cnf currently hardcodes datadir=/perf1/mysql/data which can cause startup failures if that path doesn't exist; update the deployment to make the datadir configurable instead of hardcoded by reading it from an environment variable or build-time config and substituting it into the my.cnf, or document the required path in the setup instructions; specifically modify the configuration generation step (the code that writes/installs my.cnf) to replace the literal "datadir=/perf1/mysql/data" with a templated value sourced from the build script or env var and add validation to ensure the target directory exists before starting MariaDB.hammerdb/hammerdb_scripts/mariadb/run_mariadb_tpcc.sh (1)
17-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCopy/paste error: wrong script name in usage.
The usage text references
run_mssql_tpcc.shbut this file isrun_mariadb_tpcc.sh. This will confuse users trying to understand or use the script.📝 Proposed fix
- run_mssql_tpcc.sh [-h] [-u users] [-t runlength] [-w warehouse count] [-s storage type] + run_mariadb_tpcc.sh [-h] [-u users] [-t runlength] [-w warehouse count] [-s storage type]🤖 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 `@hammerdb/hammerdb_scripts/mariadb/run_mariadb_tpcc.sh` at line 17, Update the usage string in the script's help/usage output where it currently says "run_mssql_tpcc.sh ..." to the correct script name "run_mariadb_tpcc.sh" so the usage line accurately reflects this file; locate and edit the usage/USAGE variable or echo statement in run_mariadb_tpcc.sh that prints the usage help and replace the incorrect script name.hammerdb/hammerdb_scripts/postgres/build_pg_tpcc.sh (1)
77-78:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace fixed sleep with readiness check.
The 30-second sleep after starting PostgreSQL may either waste time (if service is ready sooner) or fail to wait long enough (if startup is slow). Use a readiness polling loop instead.
🔄 Suggested readiness check pattern
systemctl start postgresql -sleep 30 +# Wait for PostgreSQL to be ready +for i in {1..30}; do + if psql -U postgres -d postgres -c "SELECT 1" &>/dev/null; then + break + fi + sleep 1 +done🤖 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 `@hammerdb/hammerdb_scripts/postgres/build_pg_tpcc.sh` around lines 77 - 78, Replace the fixed "sleep 30" after "systemctl start postgresql" with a readiness polling loop that checks service readiness (e.g., using "systemctl is-active postgresql" and/or "pg_isready") until successful or a short timeout is reached; if the check fails before the timeout, emit an error and exit non‑zero. Update the script around the "systemctl start postgresql" call to repeatedly poll at short intervals (e.g., 1s) and break when the service is active/pg_isready returns OK, otherwise fail after the timeout.hammerdb/hammerdb_scripts/postgres/run_pg_tpcc.sh (1)
100-101:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace fixed sleep with readiness check.
The 30-second sleep after restarting PostgreSQL may either waste time or fail to wait long enough. Use a readiness polling loop to verify the service is accepting connections before proceeding.
🔄 Suggested readiness check pattern
systemctl restart postgresql -sleep 30 +# Wait for PostgreSQL to be ready +for i in {1..30}; do + if psql -U postgres -d postgres -c "SELECT 1" &>/dev/null; then + break + fi + sleep 1 +done🤖 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 `@hammerdb/hammerdb_scripts/postgres/run_pg_tpcc.sh` around lines 100 - 101, Replace the fixed sleep after restarting PostgreSQL in run_pg_tpcc.sh with a readiness polling loop that repeatedly checks the server is accepting connections (e.g., using pg_isready or psql -c '\q') until it succeeds or a configurable timeout is reached; locate the restart call around "systemctl restart postgresql" and swap out "sleep 30" for the loop, log each attempt, use a short backoff between tries, and fail the script with a clear error if the timeout elapses.hammerdb/hammerdb_scripts/mssql/run_mssql_tpcc.sh (1)
92-93:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffFixed sleep duration is fragile.
As noted in previous review, the 60-second sleep may not be sufficient for MSSQL restart. A readiness check would be more reliable.
🤖 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 `@hammerdb/hammerdb_scripts/mssql/run_mssql_tpcc.sh` around lines 92 - 93, The fixed "sleep 60" in run_mssql_tpcc.sh (the echo "Waiting 60 seconds for DB to start" / sleep 60 block) is fragile; replace it with an active readiness loop that polls MSSQL until it accepts connections (for example using sqlcmd -Q "SELECT 1" or checking the TCP port with nc/tcping) with a configurable timeout and short sleep/backoff between attempts, fail if timeout elapses, and log each attempt; update the script to remove the hardcoded sleep and use that loop so the script continues only when the DB is actually ready.hammerdb/hammerdb_scripts/mssql/build_mssql_tpcc.sh (1)
61-62:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffFixed sleep duration is fragile.
As noted in previous review, the 60-second sleep may not be sufficient for MSSQL startup. A readiness check would be more reliable.
🤖 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 `@hammerdb/hammerdb_scripts/mssql/build_mssql_tpcc.sh` around lines 61 - 62, Replace the fixed "sleep 60" wait in build_mssql_tpcc.sh with a readiness loop that polls MSSQL until it is accepting connections (e.g., use sqlcmd -S <host>,<port> or a TCP check on port 1433) and only proceeds when a successful connection/response is observed or a configurable timeout expires; update the echo "Waiting 60 seconds for DB to start" messaging to reflect the polling behavior and ensure any failure path exits non‑zero after the timeout.
🧹 Nitpick comments (1)
hammerdb/hammerdb_scripts/mssql/mssql.conf (1)
1-23: ⚡ Quick winConfiguration file appears redundant with dynamic generation.
The wrapper orchestration calls
mssql_settings.shwhich usesmssql-confcommands to dynamically generate/var/opt/mssql/mssql.conf. This static file is never directly consumed by the installation flow, which could create confusion about which configuration is authoritative. If this file serves as documentation or a reference template, consider adding a comment header to clarify its purpose and relationship tomssql_settings.sh.🤖 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 `@hammerdb/hammerdb_scripts/mssql/mssql.conf` around lines 1 - 23, The mssql.conf file appears redundant because the installation flow dynamically generates /var/opt/mssql/mssql.conf via the mssql_settings.sh wrapper which invokes mssql-conf; add a clear comment header at the top of this static mssql.conf explaining that it is a reference/template only (not used at runtime), pointing to mssql_settings.sh and mssql-conf as the authoritative source, or remove the static file if it serves no purpose; reference the static file name mssql.conf and the scripts/commands mssql_settings.sh and mssql-conf in the header so readers know which is authoritative.
🤖 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 `@hammerdb/hammerdb_scripts/mariadb/build_mariadb_tpcc.sh`:
- Around line 71-73: Replace the blind restart + fixed sleep with a readiness
poll: after the existing systemctl restart mariadb.service in
build_mariadb_tpcc.sh (and mirror the same change in run_mariadb_tpcc.sh),
implement a loop that repeatedly runs mysqladmin ping (using the same root/user
credentials or socket used elsewhere) until it returns a successful response or
a timeout is reached, logging progress and failing with a clear error if the DB
never becomes available; keep the "Restarted DB" log but only emit it once
mysqladmin ping succeeds to ensure subsequent DB operations run only when
MariaDB is accepting connections.
- Line 79: The command in build_mariadb_tpcc.sh that uses a hardcoded password
("mysql -pabcdefgh -e 'drop database tpcc;'") exposes credentials in the process
list; instead create a temporary MySQL client config file and pass it with
--defaults-file to avoid putting the password on the command line: create a tmp
file with [client] section containing user and password, chmod 600, run mysql
--defaults-file="$TMP" -e 'drop database tpcc;', and ensure the script unlinks
the temp file (use a trap to remove it on exit) so credentials are not left on
disk.
In `@hammerdb/hammerdb_scripts/mssql/build_mssql_tpcc.sh`:
- Line 6: The script uses ${benchmark_run_dir} when writing
user-benchmark-name.txt but never initializes it; update the top of the script
to define benchmark_run_dir (e.g., read from an environment variable or set a
sensible default like ./runs) and add a guard that errors out if it's still
empty; ensure the directory exists (mkdir -p "${benchmark_run_dir}") before
running echo ${benchmark_name} > "${benchmark_run_dir}/user-benchmark-name.txt"
so the write cannot fail due to an undefined variable or missing directory.
In `@hammerdb/hammerdb_scripts/mssql/createdb_mssql_mnt.sh`:
- Around line 1-10: Ensure the script validates the -m argument: when parsing
the case for -m (the mountpoint variable) verify a non-empty $2 exists and after
the parsing loop add a check like if [ -z "$mountpoint" ]; then echo "Usage: ...
-m <mountpoint>" >&2; exit 1; fi so the script fails fast instead of using an
empty ${mountpoint} in SQL file paths; place the check before export mountpoint
and before any use of ${mountpoint}.
- Around line 1-35: Add a shebang and fail-fast error handling: add a top-line
shebang (bash) and enable strict mode (e.g., set -euo pipefail) so the script
exits on unbound vars or failures, validate that the parsed mountpoint variable
is non-empty before using it, run the sqlcmd here-doc as currently invoked
(/opt/mssql-tools/bin/sqlcmd -U sa -P abcdefgh <<! ) but capture its exit status
immediately afterward and if non-zero emit an error to stderr and exit with that
status; also ensure any subsequent steps (like the ALTER DATABASE ADD FILE
block) are only executed when sqlcmd succeeds.
In `@hammerdb/hammerdb_scripts/mssql/createdb_mssql.sh`:
- Line 2: Replace the hardcoded SA password used in the sqlcmd invocation (the
line starting with "/opt/mssql-tools/bin/sqlcmd -U sa -P abcdefgh") with a
secure retrieval from an environment variable (e.g., ${MSSQL_SA_PASSWORD}) in
createdb_mssql.sh and make the same change in createdb_mssql_mnt.sh and
build_mssql.tcl; additionally, add a guard that fails fast with a clear error if
MSSQL_SA_PASSWORD is not set, and consider switching to a credential store or
integrated authentication where available instead of embedding secrets in
scripts.
- Around line 3-4: The script currently runs an unconditional "DROP DATABASE
tpcc" which will error if the database does not exist; change this to use the
idempotent form "DROP DATABASE IF EXISTS tpcc" (or wrap the drop in TRY/CATCH)
so the batch does not abort on first run—update the statement in
createdb_mssql.sh (the "DROP DATABASE tpcc" line) to the IF EXISTS variant or
add proper error handling around the DROP DATABASE call.
- Around line 1-24: Add a shebang (e.g., #!/bin/bash) at the top and enable
strict error handling (use set -euo pipefail) so the script exits on failure;
ensure the block that invokes /opt/mssql-tools/bin/sqlcmd is run with error
checking and propagate non-zero exit codes instead of silently continuing (refer
to the sqlcmd invocation and the existing exit marker in the here-doc), and
optionally capture sqlcmd errors to stderr before exiting so the caller/wrapper
can detect and log failures.
In `@hammerdb/hammerdb_scripts/mssql/mssql_settings.sh`:
- Line 1: Add a shebang line to the very top of the script so the interpreter is
explicit: insert the bash shebang (#!/bin/bash) as the first line before the
existing header comment "## Setting control parameters" in mssql_settings.sh;
ensure the shebang is the file's first line so the script runs reliably under
bash when executed.
- Around line 2-22: The script runs many /opt/mssql/bin/mssql-conf invocations
(e.g., control.alternatewritethrough, control.writethrough,
memory.disablememorypressure and multiple traceflag calls) without checking
errors; update mssql_settings.sh to validate each mssql-conf command return code
(or wrap calls in a helper like run_mssql_conf) and on failure emit a clear
error message and exit non‑zero so the wrapper stops and MSSQL isn’t restarted
with partial/failed configuration; ensure the log includes which mssql-conf
subcommand failed (e.g., the specific traceflag or setting name) to aid
debugging.
In `@hammerdb/hammerdb_scripts/mssql/update_temp.sh`:
- Line 20: The scripts use a hardcoded SA password ("abcdefgh") in the sqlcmd
invocation (seen in update_temp.sh, runtest_mssql.tcl, and run_mssql_tpcc.sh);
replace that hardcoded password by reading a validated environment variable
(MSSQL_SA_PASSWORD) at the start of each script, fail fast with a clear error if
it's empty/unset, and pass the variable into the sqlcmd -P argument (and any
other places that reference the plaintext secret) so credential rotation and
secure injection via CI/runtime secrets is possible.
In `@hammerdb/hammerdb_scripts/postgres/pg_hba.conf`:
- Around line 3-5: Replace the insecure "trust" entries — specifically the
"local all all trust" and
"host all all all trust" lines — with
secure defaults: use "local all all peer" for local socket auth and
restrict remote "host" entries to localhost (e.g., "host all all
127.0.0.1/32 scram-sha-256") or explicit benchmark-only CIDRs and
authentication method "scram-sha-256" (or "md5" if SCRAM not available). Add a
top-of-file comment warning that this file is intended only for isolated
benchmark environments and must not be deployed to production or untrusted
networks. Ensure any broader host CIDRs are intentionally limited and
documented.
In `@hammerdb/hammerdb_scripts/postgres/run_pg_tpcc.sh`:
- Line 92: The CSV filename is wrong: in run_pg_tpcc.sh the echo line writing
"Run_mariadb_tpm.csv" should be changed to a Postgres-specific name (e.g.,
"Run_postgres_tpm.csv" or "Run_pg_tpm.csv") so the output file correctly
reflects this script's database; update the string in the echo command (the line
that currently contains echo
"StartTime,EndTime,Hostname,Kernel,Database,DBVer,Cpus,Memory,StorageType,Users,Tpm"
> Run_mariadb_tpm.csv) and search for any other references to
"Run_mariadb_tpm.csv" in the script to rename them consistently.
- Line 112: The script currently copies the wrong config file via the command
"cp /etc/my.cnf $benchmark_results_dir"; change this to copy the PostgreSQL
config (e.g., replace with copying "/var/lib/pgsql/data/postgresql.conf" or the
appropriate postgresql.conf on target systems) or remove the copy line if the
PostgreSQL config is already captured elsewhere; ensure you keep the destination
variable "$benchmark_results_dir" so the file is archived correctly.
In `@hammerdb/install-script`:
- Around line 269-281: The script currently downloads and immediately executes
HammerDB-3.2-Linux-x86-64-Install as root (using variables hmd_dir and the
installer name) without any integrity/authenticity checks; change the flow to
first download the installer to a temporary location, verify its checksum or
signature against a pinned digest/signature for the HammerDB release, and only
if the verification passes proceed to set permissions (chmod 755) and run the
installer (the invocation that redirects input from the generated input file).
Fail closed (exit non-zero) if verification fails and log a clear error
mentioning the installer name and expected digest.
- Line 24: The install-script assumes local wrapper variables (hmd_dir,
script_dir, run_dir) exist on the remote host and uses them (e.g., paths
referenced in hammerdb/run_hammerdb when it SSHes and runs ./install-script),
causing missing-path failures on fresh nodes; update install-script to derive or
create these paths at runtime (compute default values if env vars are unset and
mkdir -p them), or stage/copy the wrapper config and hammerdb_scripts tree to
the remote before invoking install-script; ensure all references to hmd_dir,
script_dir, and run_dir inside install-script (and affected lines around the
listed ranges) fall back to computed defaults and create the directories so
package setup and DB script staging succeed on fresh remotes.
- Around line 54-64: Hardcoded MSSQL SA password is insecure: stop setting
MSSQL_SA_PASSWORD="abcdefgh" in the script and instead accept a secret from
outside or generate and persist a per-run random value; update the install flow
so the script reads MSSQL_SA_PASSWORD from the environment or from a provided
config file/CLI argument (falling back to generating a strong random password at
runtime), export that value as MSSQL_SA_PASSWORD before starting MSSQL, and
ensure any other consumer (the MSSQL runner) reads the same env variable rather
than relying on the baked-in string.
---
Duplicate comments:
In `@hammerdb/hammerdb_scripts/mariadb/my.cnf`:
- Line 4: The my.cnf currently hardcodes datadir=/perf1/mysql/data which can
cause startup failures if that path doesn't exist; update the deployment to make
the datadir configurable instead of hardcoded by reading it from an environment
variable or build-time config and substituting it into the my.cnf, or document
the required path in the setup instructions; specifically modify the
configuration generation step (the code that writes/installs my.cnf) to replace
the literal "datadir=/perf1/mysql/data" with a templated value sourced from the
build script or env var and add validation to ensure the target directory exists
before starting MariaDB.
In `@hammerdb/hammerdb_scripts/mariadb/run_mariadb_tpcc.sh`:
- Line 17: Update the usage string in the script's help/usage output where it
currently says "run_mssql_tpcc.sh ..." to the correct script name
"run_mariadb_tpcc.sh" so the usage line accurately reflects this file; locate
and edit the usage/USAGE variable or echo statement in run_mariadb_tpcc.sh that
prints the usage help and replace the incorrect script name.
In `@hammerdb/hammerdb_scripts/mssql/build_mssql_tpcc.sh`:
- Around line 61-62: Replace the fixed "sleep 60" wait in build_mssql_tpcc.sh
with a readiness loop that polls MSSQL until it is accepting connections (e.g.,
use sqlcmd -S <host>,<port> or a TCP check on port 1433) and only proceeds when
a successful connection/response is observed or a configurable timeout expires;
update the echo "Waiting 60 seconds for DB to start" messaging to reflect the
polling behavior and ensure any failure path exits non‑zero after the timeout.
In `@hammerdb/hammerdb_scripts/mssql/run_mssql_tpcc.sh`:
- Around line 92-93: The fixed "sleep 60" in run_mssql_tpcc.sh (the echo
"Waiting 60 seconds for DB to start" / sleep 60 block) is fragile; replace it
with an active readiness loop that polls MSSQL until it accepts connections (for
example using sqlcmd -Q "SELECT 1" or checking the TCP port with nc/tcping) with
a configurable timeout and short sleep/backoff between attempts, fail if timeout
elapses, and log each attempt; update the script to remove the hardcoded sleep
and use that loop so the script continues only when the DB is actually ready.
In `@hammerdb/hammerdb_scripts/postgres/build_pg_tpcc.sh`:
- Around line 77-78: Replace the fixed "sleep 30" after "systemctl start
postgresql" with a readiness polling loop that checks service readiness (e.g.,
using "systemctl is-active postgresql" and/or "pg_isready") until successful or
a short timeout is reached; if the check fails before the timeout, emit an error
and exit non‑zero. Update the script around the "systemctl start postgresql"
call to repeatedly poll at short intervals (e.g., 1s) and break when the service
is active/pg_isready returns OK, otherwise fail after the timeout.
In `@hammerdb/hammerdb_scripts/postgres/run_pg_tpcc.sh`:
- Around line 100-101: Replace the fixed sleep after restarting PostgreSQL in
run_pg_tpcc.sh with a readiness polling loop that repeatedly checks the server
is accepting connections (e.g., using pg_isready or psql -c '\q') until it
succeeds or a configurable timeout is reached; locate the restart call around
"systemctl restart postgresql" and swap out "sleep 30" for the loop, log each
attempt, use a short backoff between tries, and fail the script with a clear
error if the timeout elapses.
---
Nitpick comments:
In `@hammerdb/hammerdb_scripts/mssql/mssql.conf`:
- Around line 1-23: The mssql.conf file appears redundant because the
installation flow dynamically generates /var/opt/mssql/mssql.conf via the
mssql_settings.sh wrapper which invokes mssql-conf; add a clear comment header
at the top of this static mssql.conf explaining that it is a reference/template
only (not used at runtime), pointing to mssql_settings.sh and mssql-conf as the
authoritative source, or remove the static file if it serves no purpose;
reference the static file name mssql.conf and the scripts/commands
mssql_settings.sh and mssql-conf in the header so readers know which is
authoritative.
🪄 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: Enterprise
Run ID: a2dcca62-c9ba-4fc3-b626-a724904e2f78
📒 Files selected for processing (26)
hammerdb/hammerdbhammerdb/hammerdb.jsonhammerdb/hammerdb_mssql.jsonhammerdb/hammerdb_scripts/mariadb/build_mariadb.tclhammerdb/hammerdb_scripts/mariadb/build_mariadb_tpcc.shhammerdb/hammerdb_scripts/mariadb/libmysqlclient.so.20hammerdb/hammerdb_scripts/mariadb/my.cnfhammerdb/hammerdb_scripts/mariadb/run_mariadb_tpcc.shhammerdb/hammerdb_scripts/mariadb/runtest_mariadb.tclhammerdb/hammerdb_scripts/mssql/build_mssql.tclhammerdb/hammerdb_scripts/mssql/build_mssql_tpcc.shhammerdb/hammerdb_scripts/mssql/createdb_mssql.shhammerdb/hammerdb_scripts/mssql/createdb_mssql_mnt.shhammerdb/hammerdb_scripts/mssql/mssql.confhammerdb/hammerdb_scripts/mssql/mssql_settings.shhammerdb/hammerdb_scripts/mssql/run_mssql_tpcc.shhammerdb/hammerdb_scripts/mssql/runtest_mssql.tclhammerdb/hammerdb_scripts/mssql/update_temp.shhammerdb/hammerdb_scripts/postgres/build_pg.tclhammerdb/hammerdb_scripts/postgres/build_pg_tpcc.shhammerdb/hammerdb_scripts/postgres/pg_hba.confhammerdb/hammerdb_scripts/postgres/postgresql.confhammerdb/hammerdb_scripts/postgres/run_pg_tpcc.shhammerdb/hammerdb_scripts/postgres/runtest_pg.tclhammerdb/install-scripthammerdb/run_hammerdb
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
hammerdb/hammerdb_scripts/mssql/createdb_mssql_mnt.sh (2)
8-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPotential infinite loop in argument parsing
If any argument other than
-mis passed,Line 8loop never shifts$1, so the script can hang indefinitely. Add a default*)branch that errors and exits (or at leastshifts).As per coding guidelines, this is a major reliability issue and should be fixed before merge.
🤖 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 `@hammerdb/hammerdb_scripts/mssql/createdb_mssql_mnt.sh` around lines 8 - 15, The argument-parsing loop in the script (the while [ $# -gt 0 ] ... case $1 in) can hang when an unrecognized flag is passed because only the -m case shifts arguments; add a default case *) that either prints an error and exits non-zero or at minimum shifts past the unknown argument so the loop progresses; update the case block handling for -m and the new *) branch to ensure $1/$2 are consumed and mountpoint is still set by the -m handler.Source: Coding guidelines
20-42:⚠️ Potential issue | 🟠 MajorAdd
-btosqlcmdto fail fast on SQL errors (createdb_mssql_mnt.sh, lines 20-42)The script uses
set -e, butsqlcmdis invoked without-b, so T-SQL errors may not causesqlcmdto exit non-zero—allowing the install flow to continue after failedCREATE DATABASE/ALTER DATABASEoperations.Suggested patch
-/opt/mssql-tools/bin/sqlcmd -U sa -P abcdefgh <<! +/opt/mssql-tools/bin/sqlcmd -b -U sa -P abcdefgh <<!🤖 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 `@hammerdb/hammerdb_scripts/mssql/createdb_mssql_mnt.sh` around lines 20 - 42, The sqlcmd invocation in createdb_mssql_mnt.sh (/opt/mssql-tools/bin/sqlcmd -U sa -P abcdefgh <<!) must pass -b so sqlcmd exits with non‑zero on T‑SQL errors; update the sqlcmd command to include the -b flag (keeping the existing -U, -P and the here‑doc delimiter '!') so any CREATE/ALTER DATABASE failures cause the script to fail fast and propagate the error to the shell (ensure the here‑doc content and exit/! delimiter remain unchanged).Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@hammerdb/hammerdb_scripts/mssql/createdb_mssql_mnt.sh`:
- Around line 8-15: The argument-parsing loop in the script (the while [ $# -gt
0 ] ... case $1 in) can hang when an unrecognized flag is passed because only
the -m case shifts arguments; add a default case *) that either prints an error
and exits non-zero or at minimum shifts past the unknown argument so the loop
progresses; update the case block handling for -m and the new *) branch to
ensure $1/$2 are consumed and mountpoint is still set by the -m handler.
- Around line 20-42: The sqlcmd invocation in createdb_mssql_mnt.sh
(/opt/mssql-tools/bin/sqlcmd -U sa -P abcdefgh <<!) must pass -b so sqlcmd exits
with non‑zero on T‑SQL errors; update the sqlcmd command to include the -b flag
(keeping the existing -U, -P and the here‑doc delimiter '!') so any CREATE/ALTER
DATABASE failures cause the script to fail fast and propagate the error to the
shell (ensure the here‑doc content and exit/! delimiter remain unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: b2255c5d-5b9e-439c-b4f0-d3565abdc78d
📒 Files selected for processing (1)
hammerdb/hammerdb_scripts/mssql/createdb_mssql_mnt.sh
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
Update hammerdb installation process, including fixing mssql
Before/After Comparison
Before: We had to upload the kit for hammerdb, and we are running the wrong version of mssql
After: The wrapper now handles loading everything it needs, and runs the proper mssql version/
Clerical Stuff
This closes #55
Relates to JIRA: RPOPC-922
Testing:
Verified mariadb, mssql, and postgres all run properly.
mariadb csv file
connection,TPM,Start_Date,End_Date
10,148610,2026-05-05T16:51:32Z,2026-05-05T17:08:40Z
20,142877,2026-05-05T17:08:50Z,2026-05-05T17:25:58Z
40,134952,2026-05-05T17:26:08Z,2026-05-05T17:43:16Z
postgres csv file
connection,TPM,Start_Date,End_Date
10,191257,2026-05-06T11:38:22Z,2026-05-06T11:55:30Z
20,186956,2026-05-06T11:55:40Z,2026-05-06T12:12:48Z
40,180997,2026-05-06T12:12:58Z,2026-05-06T12:30:06Z
mssql csv file
connection,TPM,Start_Date,End_Date
10,209219,2026-05-07T01:24:16Z,2026-05-07T01:40:24Z
20,257094,2026-05-07T01:40:34Z,2026-05-07T01:56:47Z
40,272160,2026-05-07T01:56:57Z,2026-05-07T02:13:10Z
New version has also been used in doing hammerdb runs on AWS and azure regresssions.