Skip to content

Commit 8159b4f

Browse files
committed
fix: initialize time series before concurrent chunk writes
1 parent 4569028 commit 8159b4f

2 files changed

Lines changed: 136 additions & 22 deletions

File tree

cwms/timeseries/timeseries.py

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -528,14 +528,20 @@ def store_multi_timeseries_df(
528528
DELETE_INSERT.
529529
override_protection: bool, optional, default is False
530530
A flag to ignore the protected data quality flag when storing data.
531-
multithread: bool, default is false
531+
multithread: bool, default is true
532532
Specifies whether to store chunked time series values using multiple threads.
533533
max_workers: Int, Optional, default is None
534534
It is a number of Threads aka size of pool in concurrent.futures.ThreadPoolExecutor.
535535
536536
Returns
537537
-------
538538
None
539+
540+
Raises
541+
------
542+
RuntimeError
543+
If any series fails to store. The message identifies failed series;
544+
other series may already have been stored successfully.
539545
"""
540546

541547
def store_ts_ids(
@@ -544,24 +550,21 @@ def store_ts_ids(
544550
office_id: str,
545551
version_date: Optional[datetime] = None,
546552
) -> None:
547-
try:
548-
units = data["units"].iloc[0]
549-
data_json = timeseries_df_to_json(
550-
data=data,
551-
ts_id=ts_id,
552-
units=units,
553-
office_id=office_id,
554-
version_date=version_date,
555-
)
556-
store_timeseries(
557-
data=data_json,
558-
create_as_ltrs=create_as_ltrs,
559-
store_rule=store_rule,
560-
override_protection=override_protection,
561-
multithread=multithread,
562-
)
563-
except Exception as e:
564-
print(f"Error processing {ts_id}: {e}")
553+
units = data["units"].iloc[0]
554+
data_json = timeseries_df_to_json(
555+
data=data,
556+
ts_id=ts_id,
557+
units=units,
558+
office_id=office_id,
559+
version_date=version_date,
560+
)
561+
store_timeseries(
562+
data=data_json,
563+
create_as_ltrs=create_as_ltrs,
564+
store_rule=store_rule,
565+
override_protection=override_protection,
566+
multithread=multithread,
567+
)
565568
return None
566569

567570
required_columns = ["date-time", "value", "ts_id", "units"]
@@ -577,7 +580,9 @@ def store_ts_ids(
577580
ts_data_all["ts_id"].astype(str) + ":" + ts_data_all["version_date"].astype(str)
578581
).unique()
579582

583+
errors: List[str] = []
580584
with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor:
585+
futures = {}
581586
for unique_tsid in unique_tsids:
582587
ts_id, version_date = unique_tsid.split(":", 1)
583588
if version_date != "NaT":
@@ -592,9 +597,21 @@ def store_ts_ids(
592597
(ts_data_all["ts_id"] == ts_id) & ts_data_all["version_date"].isna()
593598
]
594599
if not data.empty:
595-
executor.submit(
600+
future = executor.submit(
596601
store_ts_ids, ts_data, ts_id, office_id, version_date_dt
597602
)
603+
futures[future] = unique_tsid
604+
605+
for future in concurrent.futures.as_completed(futures):
606+
try:
607+
future.result()
608+
except Exception as e:
609+
errors.append(f"{futures[future]}: {e}")
610+
611+
if errors:
612+
raise RuntimeError(
613+
f"{len(errors)} time series failed to store:\n" + "\n".join(errors)
614+
)
598615

599616

600617
def chunk_timeseries_data(
@@ -686,7 +703,14 @@ def store_timeseries(
686703
if len(chunks) == 1 or not multithread:
687704
return api.post(endpoint, data, params)
688705

689-
actual_workers = min(max_workers, len(chunks))
706+
if max_workers <= 0:
707+
raise ValueError("max_workers must be greater than 0")
708+
709+
# A new series must exist before multiple transactions can write its data.
710+
# Complete one normal write first, then retain parallelism for the rest.
711+
_call_with_retry(api.post, endpoint, chunks[0], params)
712+
remaining_chunks = chunks[1:]
713+
actual_workers = min(max_workers, len(remaining_chunks))
690714
logging.debug(
691715
f"Storing {len(chunks)} chunks of timeseries data with {actual_workers} threads"
692716
)
@@ -698,7 +722,7 @@ def store_timeseries(
698722
with concurrent.futures.ThreadPoolExecutor(max_workers=actual_workers) as executor:
699723
future_to_chunk = {
700724
executor.submit(_call_with_retry, api.post, endpoint, chunk, params): chunk
701-
for chunk in chunks
725+
for chunk in remaining_chunks
702726
}
703727

704728
for future in concurrent.futures.as_completed(future_to_chunk):
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import threading
2+
3+
import pandas as pd
4+
import pytest
5+
6+
import cwms.timeseries.timeseries as ts
7+
8+
9+
def test_chunked_store_creates_series_before_parallel_writes(monkeypatch):
10+
created = threading.Event()
11+
lock = threading.Lock()
12+
received = []
13+
initializing = False
14+
data = {
15+
"name": "Test.Stage.Inst.15Minutes.0.Raw",
16+
"office-id": "MVP",
17+
"units": "ft",
18+
"values": [[i, i, 0] for i in range(8)],
19+
}
20+
21+
def post(endpoint, chunk, params):
22+
nonlocal initializing
23+
with lock:
24+
first = not initializing
25+
initializing = True
26+
if first:
27+
# Model the database transaction that creates a new series. Other
28+
# writes cannot use its identifier until that transaction commits.
29+
assert not created.wait(0.1)
30+
created.set()
31+
else:
32+
assert created.is_set(), "concurrent write raced series creation"
33+
assert endpoint == "timeseries"
34+
assert params["store-rule"] == "REPLACE_ALL"
35+
assert chunk["office-id"] == "MVP"
36+
with lock:
37+
received.extend(chunk["values"])
38+
39+
monkeypatch.setattr(ts.api, "post", post)
40+
ts.store_timeseries(data, chunk_size=2, store_rule="REPLACE_ALL")
41+
assert sorted(received) == data["values"]
42+
43+
44+
def test_initial_chunk_failure_stops_remaining_writes(monkeypatch):
45+
calls = []
46+
47+
def post(endpoint, chunk, params):
48+
calls.append(chunk["values"])
49+
raise ValueError("cannot create series")
50+
51+
monkeypatch.setattr(ts.api, "post", post)
52+
with pytest.raises(ValueError, match="cannot create series"):
53+
ts.store_timeseries(
54+
{
55+
"name": "Test.Stage.Inst.15Minutes.0.Raw",
56+
"office-id": "MVP",
57+
"units": "ft",
58+
"values": [[i, i, 0] for i in range(6)],
59+
},
60+
chunk_size=2,
61+
)
62+
assert calls
63+
assert all(chunk == [[0, 0, 0], [1, 1, 0]] for chunk in calls)
64+
65+
66+
def test_multi_store_reports_all_failed_series(monkeypatch):
67+
attempted = []
68+
lock = threading.Lock()
69+
data = pd.DataFrame(
70+
{
71+
"date-time": pd.to_datetime(["2025-01-01"] * 3, utc=True),
72+
"value": [1, 2, 3],
73+
"ts_id": ["good", "bad-one", "bad-two"],
74+
"units": ["ft"] * 3,
75+
}
76+
)
77+
78+
def store(data, **kwargs):
79+
with lock:
80+
attempted.append(data["name"])
81+
if data["name"].startswith("bad"):
82+
raise ValueError("write rejected")
83+
84+
monkeypatch.setattr(ts, "store_timeseries", store)
85+
with pytest.raises(RuntimeError) as error:
86+
ts.store_multi_timeseries_df(data, "MVP")
87+
assert "bad-one" in str(error.value)
88+
assert "bad-two" in str(error.value)
89+
assert "write rejected" in str(error.value)
90+
assert sorted(attempted) == ["bad-one", "bad-two", "good"]

0 commit comments

Comments
 (0)