OpenEMS Bug Report / PR — Negative energy history after restart (cumulated-energy base reset to 0)

[sthhs]

Title

Timedata: cumulated energy channels reset their base to 0 on restart when the latest-value query fails → negative values in history

Labels

bug, edge, timedata

---

Summary

On an Edge (re)start, every cumulated energy channel (_sum/*ActiveEnergy, meter/ESS energy
channels, …) re-initializes its base value baseCumulatedEnergy by querying the latest stored
value from Timedata. If that query fails transiently (DB/I-O error, contention during boot),
the value is silently treated as "no history" and the base is set to 0L. The monotonic
cumulated counter then jumps down to ~0 and continues counting up from there.

Because the history calculates a period's energy as last − first, any period that contains such a
downward jump becomes negative (a full day can show e.g. −1,649.5 kWh of "production").

The failure is non-deterministic per channel per restart: getLatestValue() runs on
CompletableFuture.supplyAsync(...) without an explicit executor, i.e. on the shared
ForkJoinPool.commonPool(). At boot, dozens of energy channels query rrd4j in parallel; under
contention some reads throw, get swallowed, and only those channels reset — so a different subset
of channels is corrupted on each restart.

Impact

• Wrong (often negative) energy values in the history / energy monitor for the affected day(s).
• Corruption is persistent: the wrong base is written back into Timedata (rrd4j), so the
  downward step survives in the stored series and keeps poisoning every query that spans it.
• Affects any component using CalculateEnergyFromPower with the rrd4j Timedata backend.
• Not configurable around — requires a code change.

Affected code

1. io.openems.edge.timedata.rrd4j ·
   Rrd4jReadHandler.getLatestValue(...) — the try (…rrd4j read…) { } catch (Exception e) swallows
   any DB/I-O error into return Optional.empty(); (around line 577 on develop).
   Note: the channel-not-found branch above it already rethrows a CompletionException — only the
   database read branch still swallows.
2. io.openems.edge.timedata.api ·
   CalculateEnergyFromPower.initializeCumulatedEnergyFromTimedata() — on the retry-handler GIVE-UP
   branch it does this.baseCumulatedEnergy = 0L; (around line 172 on develop). An empty result
   (whether genuine or produced by config.json is cleared #1) is likewise mapped to 0L.

---

Steps to reproduce

1. Run an Edge with the rrd4j Timedata backend and components that derive energy from power
   (CalculateEnergyFromPower), e.g. _sum production/consumption energy.
2. Let it accumulate a non-trivial cumulated value (e.g. _sum/ProductionActiveEnergy in the MWh
   range).
3. Restart the Edge (or trigger a transient rrd4j read error during the init window — easiest to
   observe on a system with many energy channels initializing concurrently at boot).
4. Observe a subset of energy channels whose stored cumulated value drops to ~0 at the restart
   timestamp, and a negative day-value in the history for that day.

---

Root cause

restart
  └─ CalculateEnergyFromPower.initializeCumulatedEnergyFromTimedata()
       └─ timedata.getLatestValue(channel)              // async on ForkJoinPool.commonPool()
            └─ Rrd4jReadHandler.getLatestValue()
                 └─ transient rrd4j read throws
                      └─ catch (Exception e) → return Optional.empty()   // (1) error == "no data"
       └─ whenComplete: result empty  →  baseCumulatedEnergy = 0L        // (2) "no data" == reset to 0
  └─ cumulated counter jumps to ~0 and is persisted
  └─ history period containing the jump:  last − first  <  0

Two independent weaknesses combine: (1) an error is indistinguishable from "no history", and
(2) "no history" is treated as "start from 0" even for a channel that demonstrably had a large
history a moment earlier.

---

Fix

Two minimal, independent changes. (A) makes transient read errors propagate into the existing
retry path instead of masquerading as "no data"; (B) makes the ultimate give-up safe by never
destroying a counter to 0.

Both diffs apply cleanly to develop @ 79ba3fad.

A) Rrd4jReadHandler.getLatestValue — propagate read errors instead of swallowing

CompletionException is already imported in this file, and the channel-not-found branch above
already uses this exact pattern — this aligns the database-read branch with it.

diff --git a/io.openems.edge.timedata.rrd4j/src/io/openems/edge/timedata/rrd4j/Rrd4jReadHandler.java b/io.openems.edge.timedata.rrd4j/src/io/openems/edge/timedata/rrd4j/Rrd4jReadHandler.java
index 063ab7649..5cdd5a8cc 100644
--- a/io.openems.edge.timedata.rrd4j/src/io/openems/edge/timedata/rrd4j/Rrd4jReadHandler.java
+++ b/io.openems.edge.timedata.rrd4j/src/io/openems/edge/timedata/rrd4j/Rrd4jReadHandler.java
@@ -575,7 +575,12 @@ public class Rrd4jReadHandler {
 
 				return Optional.empty();
 			} catch (Exception e) {
-				return Optional.empty();
+				// Do NOT swallow transient DB/I-O errors into Optional.empty(): the caller
+				// (CalculateEnergyFromPower) cannot distinguish "no history" from "query
+				// failed" and resets the cumulated-energy base to 0, corrupting the counter.
+				// Propagate instead, so the caller's retry logic can handle it.
+				this.log.warn("Unable to query [" + channelAddress + "] from RRD4j: " + e.getMessage());
+				throw new CompletionException("Unable to query [" + channelAddress + "] from RRD4j", e);
 			}
 		});
 	}

After this change, Optional.empty() from getLatestValue means only genuinely-no-stored-value
(truly new channel → base 0 is correct there), and a transient failure surfaces as a throwable
that CalculateEnergyFromPower's TimedataQueryRetryHandler already knows how to retry.

B) CalculateEnergyFromPower — do not fall back to 0 on give-up

diff --git a/io.openems.edge.timedata.api/src/io/openems/edge/timedata/api/utils/CalculateEnergyFromPower.java b/io.openems.edge.timedata.api/src/io/openems/edge/timedata/api/utils/CalculateEnergyFromPower.java
index a3100f32e..bbc9a7a52 100644
--- a/io.openems.edge.timedata.api/src/io/openems/edge/timedata/api/utils/CalculateEnergyFromPower.java
+++ b/io.openems.edge.timedata.api/src/io/openems/edge/timedata/api/utils/CalculateEnergyFromPower.java
@@ -168,8 +168,12 @@ public class CalculateEnergyFromPower {
 							return;
 						}
 
-						this.state = State.CALCULATE_ENERGY_OPERATION;
-						this.baseCumulatedEnergy = 0L;
+						// GIVE-UP after the retry window: do NOT fall back to 0. Resetting the
+						// base to 0 corrupts the monotonic cumulated-energy counter (the value
+						// jumps down), which produces negative day-values in the history. Leave
+						// the base undefined (channel stays null = harmless gap) and keep
+						// retrying on later cycles so it self-heals once Timedata is readable.
+						this.state = State.TIMEDATA_QUERY_NOT_STARTED;
 						return;
 					}
 

With (A) in place, the give-up branch is reached only for a persistent failure. Keeping the base
null makes calculateEnergy() publish null (a gap) instead of a destructive 0, and returning
to TIMEDATA_QUERY_NOT_STARTED re-queries on later cycles so the channel self-heals once Timedata
is readable again. The extra async query per cycle is negligible and stops the moment it succeeds.

Note on the empty-result branch: The else { this.baseCumulatedEnergy = 0L; } further down
(when the future completes with Optional.empty()) is intentionally kept — after fix (A) an
empty result genuinely means "new channel with no history", where 0 is correct.

Optional hardening (defense-in-depth, not required for the fix)

The core fix is (A)+(B). The following two changes are complements, not replacements — they add
monotonicity safeguards and observability, and apply cleanly on top of (A)+(B). Note: a pure
in-memory monotonicity guard cannot catch the restart reset, because at fresh startup there is no
trusted in-memory reference — the trusted reference is exactly the read that (A) makes reliable.

C) CalculateEnergyFromPower — never lower an already-established base
Cumulated energy is monotonic by design, so a queried base must never reduce one already known in
this run. Also stop resetting to 0 on a type-parse error.

diff --git a/io.openems.edge.timedata.api/src/io/openems/edge/timedata/api/utils/CalculateEnergyFromPower.java b/io.openems.edge.timedata.api/src/io/openems/edge/timedata/api/utils/CalculateEnergyFromPower.java
index a3100f32e..410fc49b5 100644
--- a/io.openems.edge.timedata.api/src/io/openems/edge/timedata/api/utils/CalculateEnergyFromPower.java
+++ b/io.openems.edge.timedata.api/src/io/openems/edge/timedata/api/utils/CalculateEnergyFromPower.java
@@ -178,9 +178,14 @@ public class CalculateEnergyFromPower {
 
 					if (cumulatedEnergyOpt.isPresent()) {
 						try {
-							this.baseCumulatedEnergy = TypeUtils.getAsType(OpenemsType.LONG, cumulatedEnergyOpt.get());
+							final var queried = TypeUtils.getAsType(OpenemsType.LONG, cumulatedEnergyOpt.get());
+							// Cumulated energy is monotonic: never lower an already established base
+							// (defense-in-depth against a regressing or garbled stored value).
+							this.baseCumulatedEnergy = this.baseCumulatedEnergy == null //
+								? queried //
+								: Math.max(this.baseCumulatedEnergy, queried);
 						} catch (IllegalArgumentException e) {
-							this.baseCumulatedEnergy = 0L;
+							// keep an established base; otherwise leave undefined (do NOT reset to 0)
 						}
 					} else {
 						this.baseCumulatedEnergy = 0L;

D) Rrd4jReadHandler.queryHistoricEnergy — warn on a negative period (do not hide it)
A negative last − first on a monotonic channel is a definitive corruption indicator. Log it for
observability; do not clamp it to 0 (clamping would silently serve wrong-but-positive numbers).

diff --git a/io.openems.edge.timedata.rrd4j/src/io/openems/edge/timedata/rrd4j/Rrd4jReadHandler.java b/io.openems.edge.timedata.rrd4j/src/io/openems/edge/timedata/rrd4j/Rrd4jReadHandler.java
index 063ab7649..ea42f967f 100644
--- a/io.openems.edge.timedata.rrd4j/src/io/openems/edge/timedata/rrd4j/Rrd4jReadHandler.java
+++ b/io.openems.edge.timedata.rrd4j/src/io/openems/edge/timedata/rrd4j/Rrd4jReadHandler.java
@@ -427,6 +427,12 @@ public class Rrd4jReadHandler {
 						if (Double.isNaN(first) || Double.isNaN(last)) {
 							return JsonNull.INSTANCE;
 						}
+						if (last - first < 0) {
+							// A cumulated energy channel must be monotonic; a negative period value
+							// indicates a corrupted (reset) base value in the stored series.
+							this.log.warn("Non-monotonic cumulated channel [" + channelAddress + "]: last=" + last
+								+ " < first=" + first + " => data corruption suspected");
+						}
 						return new JsonPrimitive(last - first);
 					} catch (Exception e) {
 						if (debugMode) {

Why not a value >= 0 check? It would never fire: the stored cumulated value stays >= 0 (it
is reset to 0 and counts up). The negativity only appears in the derived last − first of the
history, so a non-negative guard on the channel value does not match this failure mode.

Further option: run getLatestValue on a dedicated executor instead of
ForkJoinPool.commonPool() to remove the boot-time contention that triggers the transient failures
in the first place. Reduces how often the retry path is exercised; not necessary for correctness.

---

Suggested tests

• CalculateEnergyFromPower: simulate getLatestValue completing exceptionally after the retry
  window → assert the channel value stays null and baseCumulatedEnergy is not set to 0,
  and that a subsequent successful query adopts the stored base.
• CalculateEnergyFromPower: getLatestValue → Optional.of(1_000_000L) → base adopts 1,000,000,
  not 0.
• Rrd4jReadHandler.getLatestValue: a read that throws → the returned future completes
  exceptionally (not Optional.empty()); a database with only NaN / no DB → Optional.empty().

---

Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

❗ There is a different number of reports uploaded between BASE (b0933dd) and HEAD (c90b300). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (b0933dd)	HEAD (c90b300)
java	1	0

Additional details and impacted files

@@              Coverage Diff               @@
##             develop    #4051       +/-   ##
==============================================
- Coverage      55.46%   26.19%   -29.26%     
==============================================
  Files           3931      548     -3383     
  Lines         180759    16530   -164229     
  Branches       15077     2916    -12161     
==============================================
- Hits          100239     4329    -95910     
+ Misses         76550    12013    -64537     
+ Partials        3970      188     -3782     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.