USACE / cwms-data-api

Corps Water Management System RESTful Data Service
MIT License
11 stars 14 forks source link

Time Series retrieval fails when there is no data #157

Closed adamkorynta closed 3 months ago

adamkorynta commented 2 years ago

Received the following from the server:

Request{method=GET, url=http://localhost:11526/cwms-data/timeseries?unit=SI&timezone=UTC&name=TT983530.Stage.Inst.15Minutes.0.Ccp-Rev&pageSize=100&end=2014-03-30T07%3A00%3A00Z&office=NAB&begin=2014-02-01T08%3A00%3A00Z, headers=[accept:application/json;version=2]} Error code: 500 {"message":"System Error","incidentIdentifier":"4698309580242637269","details":{}}

The TSID was stored with an undefined interval offset. At the time of retrieval there have been no values stored to the time series. I was expecting a specific error for no data found.

rripken commented 2 years ago

I'm sort of surprised you got a 500. I would have thought that you'd get back a timeseries with an empty values array. Or maybe I'm not understanding the setup.

adamkorynta commented 2 years ago

Empty values array would be fine too.

rripken commented 2 years ago

Adam helped me track this down further.

The 500 code is because the server-side caught an exception.

Here is a unit test that can trigger this error: ` @Test public void testCreateUndefOffset() throws Exception { String officeId = "LRL";

    try(Connection connection = getConnection(); DSLContext dsl = getDslContext(connection, officeId))
    {
        CwmsDbTs tsDao = CwmsDbServiceLookup.buildCwmsDb(CwmsDbTs.class, connection);

        String tsId = "RYAN3.Stage.Inst.5Minutes.0.ZSTORE_TS_TEST" + Calendar.getInstance().get(Calendar.MILLISECOND);
        if(!locationExists(connection, "RYAN3"))
        {
            storeLocation(connection, officeId, "RYAN3");
        }

        int utcOffsetMinutes = Integer.MAX_VALUE;
        Number intervalForward = null;
        Number intervalBackward = null;
        boolean versionedFlag = false;
        boolean activeFlag = true;
        BigInteger tsCode = tsDao.createTsCodeBigInteger(connection, officeId, tsId, utcOffsetMinutes,
                intervalForward, intervalBackward, versionedFlag, activeFlag);

        TimeSeriesDaoImpl dao = new TimeSeriesDaoImpl(dsl);

        final ZonedDateTime begin = ZonedDateTime.parse("2021-06-21T08:00:00-07:00[PST8PDT]");

        final ZonedDateTime end = ZonedDateTime.parse("2021-06-28T08:00:00-07:00[PST8PDT]");
        TimeSeries elevTS = dao.getTimeseries(null, 400, tsId,
                officeId, "SI", null, begin, end,
                ZoneId.of("UTC"));
    }
}

`

The pl/sql raises an error and causes a massive stack trace. In my opinion this is the juicy bit:

` ORA-06512: at "CWMS_20.CWMS_TS", line 413 ORA-06512: at "CWMS_20.CWMS_ERR", line 59 ORA-06512: at "CWMS_20.CWMS_TS", line 475 ORA-20998: ERROR: Interval offset (27071460) must be less than interval (5) ORA-06512: at "CWMS_20.CWMS_ERR", line 59 ORA-06512: at "CWMS_20.CWMS_TS", line 413 ORA-06512: at "CWMS_20.CWMS_TS", line 2361 ORA-06512: at "CWMS_20.CWMS_TS", line 2552 ORA-06512: at "CWMS_20.CWMS_TS", line 3258 ORA-06512: at "CWMS_20.CWMS_TS", line 3308 ORA-06512: at "CWMS_20.CWMS_TS", line 3352 ORA-06512: at line 1

at oracle.jdbc.driver.T4CTTIoer11.processError(T4CTTIoer11.java:513) ... 114 more `

Here I've added the function/procedure names: ORA-06512: at "CWMS_20.CWMS_TS", line 413 top_of_interval_utc ORA-06512: at "CWMS_20.CWMS_TS", line 2361 setup_retrieve ORA-06512: at "CWMS_20.CWMS_TS", line 2552 build_retrieve_ts_query ORA-06512: at "CWMS_20.CWMS_TS", line 3258 retrieve_ts_out ORA-06512: at "CWMS_20.CWMS_TS", line 3308 retrieve_ts_out ORA-06512: at "CWMS_20.CWMS_TS", line 3352 retrieve_ts_out_tab

In this test the interval_offset for the time_series is 2147483647. I've confirmed this by searching the database for the ts and verifying the value.

The setup_retrieve function looks for that constant interval_offset and handles it by calling top_of_interval_utc with null. top_of_interval_utc handles null offsets by calling a different procedure

if p_interval_offset is null then l_interval_offset := interval_offset_minutes(p_date_time, l_interval); else l_interval_offset := interval_offset_minutes(p_interval_offset); end if;

The two-arg version of interval_offset_minutes is sort of curious. It returns an offset that varies according to the start-date. In my test the interval of the timeseries was 5 minutes. For Adam's the interval was 15 minutes.

In either case the code heads into this section: when l_interval_minutes in (1, 2, 3, 4, 5, 6, 8, 10, 12, 15, 20, 30) -- minutes or l_interval_minutes in (60, 120, 180, 240, 360, 480, 720) -- hours or l_interval_minutes in (1440, 2880, 4320, 5760, 7200, 8640, 10080) -- days +1Week then l_millis := cwms_util.to_millis(cast(p_date_time as timestamp)); l_interval_millis := l_interval_minutes * c_millis_in_minute; l_offset_millis := l_millis - mod(l_millis, l_interval_millis); l_offset_minutes := l_offset_millis / c_millis_in_minute;

In my test case the start-time was "2021-06-21T08:00:00-07:00[PST8PDT]" which, when converted to UTC and then turned into timestamp millis becomes: 1624287600000

dividing by milli_in_minute produces 27071460 which is exactly the constant shown in the error message.

I suspect that top_of_interval_utc needs to be modified to handle interval offsets that exceed the interval - especially when the interval_offset is calculated from the two-arg version of interval_offset_minutes.

I'm not comfortable taking this further without someone who is much better versed in this portion of CWMS_TS.

rripken commented 2 years ago

Note that in conversations with the database team its possible the underlying issue has already been fixed in a newer schema. I will try to get a newer database to test with and verify.

DanielTOsborne commented 2 years ago

The TSID was stored with an undefined interval offset.

Since a recent CWMS schema update (20.x I think), not having a defined interval offset has broken a lot of stuff, not just in RADAR. We got hit by that multiple times. That, and not having a timezone specified in the location also breaks stuff.

MikeNeilson commented 2 years ago

Yeah, it's was fairly pervasive and we're moving away from "if there's a timezone it's local regular" so the later 21.x database should have less of an issue.