perf: batch InfluxDB queries and consolidate noisy log warnings#38
Open
jarsarasty wants to merge 2 commits into
Open
perf: batch InfluxDB queries and consolidate noisy log warnings#38jarsarasty wants to merge 2 commits into
jarsarasty wants to merge 2 commits into
Conversation
- Replace per-profile InfluxDB round-trips with one query per measurement group; per-profile fallback on failure - Consolidate repeated "not a recognised KPI field" and "negative values" per-column WARNINGs into single summary lines (first 10 shown; full list at DEBUG) Correctness fixes: - multiplier=0.0 was coerced to 1.0 via boolean short-circuit; now uses explicit None check in both loading paths - _load_from_dataframes no longer processes DataFrames that already failed validation (non-DatetimeIndex, NaN); previously stored NaN-contaminated TimeSeries silently - Duplicate Empty DataFrame error removed from load loop - NULL rows in batch results now raise instead of propagating NaN
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves time series ingestion performance and log signal quality by batching InfluxDB profile loads (one query per measurement group) and consolidating per-column validation warnings into summary lines.
Changes:
- Batch InfluxDB queries by grouping compatible
InfluxDBProfiles and splitting results back per profile, with per-profile fallback on batch failure. - Consolidate noisy per-column warnings (unrecognized KPI fields, negative values) into single summarized warnings with DEBUG-only full lists.
- Strengthen correctness around multiplier handling (
0.0preserved), validation gating (don’t process invalid DataFrames), and NULL handling in batch results.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| unit_test/test_kpi_calculator.py | Updates/extends validation tests for consolidated warnings and single error emission. |
| unit_test/test_database_time_series_loader.py | Adds unit/integration coverage for batching/grouping and batch fallback behaviors. |
| src/kpicalculator/adapters/time_series_manager.py | Skips processing invalid DataFrames and introduces consolidated warning summarization helper. |
| src/kpicalculator/adapters/database_time_series_loader.py | Implements profile batching/grouping, batch query execution, and per-profile extraction from batch results. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+265
to
+267
| self.logger.debug( | ||
| f"Full list of columns that {verb}:\n" + "\n".join(f" {c}" for c in cols) | ||
| ) |
Comment on lines
+261
to
+264
| # Security validation per profile; credentials from representative (same host/port/db) | ||
| for profile in profiles: | ||
| self._get_credentials_for_profile(profile) | ||
| credentials = self._get_credentials_for_profile(representative) |
Group InfluxDB profiles sharing the same measurement into a single SELECT query instead of N individual queries. Falls back to per-profile loading on batch failure or NULL/NaN rows from non-overlapping timestamps. Consolidate repeated per-asset log warnings into a single summary line with a DEBUG-gated full list. Also: - Widen TimeSeriesDataProtocol.profile_data_list to Sequence[Sequence[object]] - Add _apply_multiplier helper (deduplicates multiplier logic) - Add _rows_to_dataframe helper (deduplicates DataFrame construction) - Add field_index_map for O(1) field lookup replacing O(n) list.index() - Hard-error on database name mismatch instead of silent fallback - Warn on duplicate measurement.field in same batch group - Warn on multiplier=0.0
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
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.
Correctness fixes: