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
Use a single health log table #15157
Conversation
What has changed with the PR:
|
database/sqlite/sqlite_aclk_alert.c
Outdated
@@ -307,19 +274,15 @@ void aclk_push_alert_event(struct aclk_sync_host_config *wc) | |||
|
|||
buffer_sprintf(sql, "select aa.sequence_id, hl.unique_id, hl.alarm_id, hl.config_hash_id, hl.updated_by_id, hl.when_key, " \ | |||
" hl.duration, hl.non_clear_duration, hl.flags, hl.exec_run_timestamp, hl.delay_up_to_timestamp, hl.name, " \ | |||
" hl.chart, hl.family, hl.exec, hl.recipient, hl.source, hl.units, hl.info, hl.exec_code, hl.new_status, " \ | |||
" hl.chart, hl.family, hl.exec, hl.recipient, ha.source, hl.units, hl.info, hl.exec_code, hl.new_status, " \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we are inserting a field from ha
among fields from hl
table. To simplify developer read and probably database engine, I suggest to keep fields from the same table together.
database/sqlite/sqlite_health.c
Outdated
@@ -689,12 +660,11 @@ void sql_check_removed_alerts_state(char *uuid_str) | |||
/* Health related SQL queries | |||
Load from the health log table | |||
*/ | |||
#define SQL_LOAD_HEALTH_LOG(guid) "SELECT hostname, unique_id, alarm_id, alarm_event_id, config_hash_id, updated_by_id, updates_id, when_key, duration, non_clear_duration, flags, exec_run_timestamp, delay_up_to_timestamp, name, chart, family, exec, recipient, source, units, info, exec_code, new_status, old_status, delay, new_value, old_value, last_repeat, class, component, type, chart_context, transition_id FROM health_log_%s group by alarm_id having max(alarm_event_id);", guid | |||
#define SQL_LOAD_HEALTH_LOG "SELECT hl.unique_id, hl.alarm_id, hl.alarm_event_id, hl.config_hash_id, hl.updated_by_id, hl.updates_id, hl.when_key, hl.duration, hl.non_clear_duration, hl.flags, hl.exec_run_timestamp, hl.delay_up_to_timestamp, hl.name, hl.chart, hl.family, hl.exec, hl.recipient, ah.source, hl.units, hl.info, hl.exec_code, hl.new_status, hl.old_status, hl.delay, hl.new_value, hl.old_value, hl.last_repeat, ah.class, ah.component, ah.type, hl.chart_context, hl.transition_id FROM health_log hl, alert_hash ah where hl.config_hash_id = ah.hash_id and host_id = @host_id group by hl.alarm_id having max(hl.alarm_event_id);" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like one of previous comment, I suggest to reorganize fields per table.
database/sqlite/sqlite_health.c
Outdated
@@ -1208,7 +1187,7 @@ int sql_health_get_last_executed_event(RRDHOST *host, ALARM_ENTRY *ae, RRDCALC_S | |||
return ret; | |||
} | |||
|
|||
#define SQL_SELECT_HEALTH_LOG(guid) "SELECT hostname, unique_id, alarm_id, alarm_event_id, config_hash_id, updated_by_id, updates_id, when_key, duration, non_clear_duration, flags, exec_run_timestamp, delay_up_to_timestamp, name, chart, family, exec, recipient, source, units, info, exec_code, new_status, old_status, delay, new_value, old_value, last_repeat, class, component, type, chart_context, transition_id FROM health_log_%s WHERE 1=1 ", guid | |||
#define SQL_SELECT_HEALTH_LOG "SELECT hl.unique_id, hl.alarm_id, hl.alarm_event_id, hl.config_hash_id, hl.updated_by_id, hl.updates_id, hl.when_key, hl.duration, hl.non_clear_duration, hl.flags, hl.exec_run_timestamp, hl.delay_up_to_timestamp, hl.name, hl.chart, hl.family, hl.exec, hl.recipient, ah.source, hl.units, hl.info, hl.exec_code, hl.new_status, hl.old_status, hl.delay, hl.new_value, hl.old_value, hl.last_repeat, ah.class, ah.component, ah.type, hl.chart_context, hl.transition_id FROM health_log hl, alert_hash ah WHERE hl.config_hash_id = ah.hash_id and host_id = @host_id " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you accept suggestion to reorganize, please, reorganize this too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR is working as expected, LGTM!
Summary
This PR changes health to use a single
health_log
table, instead of a table for each host.The PR will update database to version 9 and will move data from all the
health_log_XXXX
tables to the new one. A newhost_id
column is added, which is the host'shost_uuid
.Test Plan
After this PR all of health functionality (including cleaning up and cloud connectivity) should remain as before.
Additional Information
For users: How does this change affect me?