Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bootstrap.sh improvements #10490

Merged

Conversation

nirbosl
Copy link
Contributor

@nirbosl nirbosl commented Feb 25, 2025

Description:

  • Simplified process tracking by removing the processing_files associative array and relying on the pids array for job tracking
  • Fixed an issue where all child processes would print out the same PID as theirs, which was in fact the script parent-group-ID; each process now properly prints its own job PID, also resulting in PGAPPNAME reflecting the correct PID in the DB COPY jobs.
  • Extended the validate_special_files function with additional validation logic
    • Fixes an issue of an error thrown for special files validation when re-running the script
  • Improved robustness of file validation checks
  • Enhanced the import_file function with additional error handling and verification steps
  • Improved import reliability
  • Improved code organization
  • Maintained the same overall script structure and core functionality
  • Converted DECOMPRESS_FLAGS into an array and added proper array handling for it; This allows to resolve a shellcheck/codacy smell regarding the var not being double-quoted to prevent globbing/splitting of the value.

Notes for reviewer:
The changes to the script's code are due to an issue a customer ran into. I was not able to reproduce the issue they ran into (valid file being imported but post-import row-count returns incorrectly then counts again and increments by the amount of rows it counts over and over).

I have focused on potential weaknesses I've identified that could potentially result in such edge-cases, and have refactored and improved their code to be more robust and safer in order to avoid such issues in the future.

I have repeat-tested the script using a dataset that included two large table file parts of each large table, and all small tables in the following two scnearios:

  • Single run import with no interruptions (good path)
  • Started an import, let it complete several files successfully then stop the script and re-run it (to validate resumption logic and proper skip of already-imported files)

All repeat tests in both scenarios finished successfully.

I have then started a local mirror-importer against the bootstrapped DB and it started cleanly and resumed sync from September 18th timestamps, which is the date this data export was taken on (0.113.2).

Checklist

  • Documented (Code comments)
  • Tested (Actual runs of the script, described above in the Notes section.)

Signed-off-by: Nir Ben-Or <[email protected]>
@nirbosl nirbosl requested a review from a team as a code owner February 25, 2025 18:02
Copy link

codecov bot commented Feb 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

see 165 files with indirect coverage changes

@steven-sheehy steven-sheehy added bug Type: Something isn't working database Area: Database enhancement Type: New feature and removed bug Type: Something isn't working labels Feb 25, 2025
@steven-sheehy steven-sheehy added this to the 0.125.0 milestone Feb 25, 2025
Copy link
Contributor

@jnels124 jnels124 left a comment

Choose a reason for hiding this comment

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

LGTM

…necessary 'basename' command call

Signed-off-by: Nir Ben-Or <[email protected]>
…ue execution; Added a new conditional block to handle a fallback boundary-check if all three full-row-count retries fail

Signed-off-by: Nir Ben-Or <[email protected]>
steven-sheehy
steven-sheehy previously approved these changes Feb 28, 2025
Signed-off-by: Nir Ben-Or <[email protected]>
@steven-sheehy steven-sheehy merged commit 5993d94 into hiero-ledger:main Mar 3, 2025
31 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Area: Database enhancement Type: New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants