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

Add MariaDB dumpfile fetch and import. #11

Closed
wants to merge 1 commit into from
Closed

Add MariaDB dumpfile fetch and import. #11

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Aug 26, 2021

Draft request at this stage as preview hoping for comments, on style and substance, and expecting more integration work to be needed.

Closes #7 #5 (for MariaDB only)

But does not yet address #6

@gregcorbett gregcorbett requested a review from a team September 6, 2021 14:19
[logs]
# set to 'info' for step-by-step summary
# only 'error' or 'info' are implemented
level=error
Copy link
Member

Choose a reason for hiding this comment

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

What's the output like when the logging level is error? The current monitoring assumes no output on a successful run.

Copy link
Author

Choose a reason for hiding this comment

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

My reading is that the current monitoring requires "completed ok" as the final line of output from a successful run. Any other condition is a failure.
I am re-working the logging to allow specification of a log file to which info/debug/error output is written while "completed ok" or an error message goes to stdout.

Copy link
Member

Choose a reason for hiding this comment

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

so "completed ok" currently goes to the syslog via the logger command, whereas the monitoring checks the log produced by the cron job that runs the import process hourly and assumes the underlying 1_runDbUpdate.sh produces no output to stdout or stderr on a successful import.

You could have the logging report at the debug level rather than info - so if something is going wrong with the process one can switch the more verbose mode on.

Copy link
Author

Choose a reason for hiding this comment

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

:lightbulb: 1_runDbUpdate.sh has an internal bash function 'logger' which (appears to me) is hardwired to output to a local file, so not to syslog. How does the output end up in syslog?

In any case, I've changed it so that stdout should be empty and "completed ok", or error message, will go in the specified log file, with return status as appropriate.

Copy link
Member

Choose a reason for hiding this comment

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

ah yes, you're right - I mistook logger for the pre defined function and not one that was defined in the script itself. And I've only just discovered updateLog.txt is even a thing...

error messages should go to stderr as well, I'm not convinced of the need to have two log files - one for the script and one for the cron.

@gregcorbett
Copy link
Member

Is dbImport.pwd intended to be used for the configuration of other mysql options? or solely for the password?

It's referenced as mysqlOptionsPath in the code, but the templates name suggests its only for the password.

@gregcorbett
Copy link
Member

codacy has highlighted somethings as well (though the README issues probably keep the file self consistent, so if you wanted to fix them, please do it in a standalone PR.

@gregcorbett gregcorbett requested a review from a team September 16, 2021 07:20
@ghost
Copy link
Author

ghost commented Oct 4, 2021

Is dbImport.pwd intended to be used for the configuration of other mysql options? or solely for the password?

It's referenced as mysqlOptionsPath in the code, but the templates name suggests its only for the password.

Yes, that's a bit confusing. There's no intent, but it is possible and there seems no reason to restrict it. I have renamed to a .cnf_template suffix and included the user so that all mysql options arrive through that route. And expanded the comments in config.ini.

@ghost ghost marked this pull request as ready for review October 5, 2021 11:39
@ghost
Copy link
Author

ghost commented Oct 6, 2021

After discussion:

  1. Add ISO 8601 timestamp to the logged output
  2. Implement failure retry with delay backoff

With optional .zip file inflation.
@ghost
Copy link
Author

ghost commented Nov 4, 2021

Closing for now as I'll be force pushing some intermediate/incomplete/untested updates.

@ghost ghost closed this Nov 4, 2021
This pull request was closed.
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.

Enable for MariaDB failover
1 participant