Repository: reframe-hpc/reframe

Issue:
Extend the `?` session selection syntax with support for time periods
Currently, we cannot either select sessions based on a time period or a filter expression using the `?` syntax. For the latter, not being able to filter also on time period is problematic, as it requires us to scan the whole DB for sessions. This can be time consuming. We propose to extend the `?` with a time-period component that would limit additionally the query based on the time period. This will considerably speed up session queries based on arbitrary session information as we will only be decoding a subset of the stored sessions.

Hints from issue comments:

Solution patch:
diff --git a/docs/manpage.rst b/docs/manpage.rst
index e090a38bd..7002d6a1a 100644
--- a/docs/manpage.rst
+++ b/docs/manpage.rst
@@ -1382,6 +1382,15 @@ The syntax for selecting sessions or test cases can take one of the following fo
 2. ``<select> := ?<session_filter>``: A valid Python expression on the available session information including any user-specific session extras (see also :option:`--session-extras`), e.g., ``?'xyz=="123"'``.
    In this case, the testcases from all sessions matching the filter will be retrieved.
 3. ``<select> := <time_period>``: A time period specification (see below for details).
+4. ``<select> := <time_period>?<session_filter>``: This is a variation of option (2), where sessions are filtered also by the time period.
+   This syntax is useful for limiting the query scope and therefore reducing query times.
+
+
+.. note::
+   .. versionchanged:: 4.8
+
+      Support for scoping the session filter queries by a time period was added.
+
 
 Time periods
 ^^^^^^^^^^^^
@@ -1505,13 +1514,13 @@ Note that parts that have a grammar defined elsewhere (e.g., Python attributes a
    <cmpspec> ::= (<select> "/")? <select> "/" <aggr> "/" <cols>
    <aggr> ::= <aggr_fn> ":" <cols>
    <aggr_fn> ::= "first" | "last" | "max" | "min" | "mean" | "median"
-   <cols> ::= <extra_cols> | <explicit_cols>
+   <cols> ::= <extra_cols> | <explicit_cols> | E
    <extra_cols> ::= ("+" <attr>)+
    <explicit_cols> ::= <attr> ("," <attr>)*
    <attr> ::= /* any Python attribute */
    <select> ::= <session_uuid> | <session_filter> | <time_period>
    <session_uuid> ::= /* any valid UUID */
-   <session_filter> ::= "?" <python_expr>
+   <session_filter> ::= (<time_period>)? "?" <python_expr>
    <python_expr> ::= /* any valid Python expression */
    <time_period> ::= <timestamp> ":" <timestamp>
    <timestamp> ::= ("now" | <abs_timestamp>) (("+" | "-") <number> ("w" | "d" | "h" | "m"))?
diff --git a/reframe/frontend/reporting/storage.py b/reframe/frontend/reporting/storage.py
index f8bd8584b..13e9bc9da 100644
--- a/reframe/frontend/reporting/storage.py
+++ b/reframe/frontend/reporting/storage.py
@@ -237,6 +237,7 @@ def _decode_sessions(self, results, sess_filter):
             r'\"session_info\":\s+(?P<sess_info>\{.*?\})'
         )
 
+        @time_function
         def _extract_sess_info(s):
             return sess_info_patt.search(s).group('sess_info')
 
@@ -318,7 +319,11 @@ def _fetch_testcases_from_session(self, selector,
                                       name_patt=None, test_filter=None):
         query = 'SELECT uuid, json_blob from sessions'
         if selector.by_session_uuid():
-            query += f' WHERE uuid == "{selector.value}"'
+            query += f' WHERE uuid == "{selector.uuid}"'
+        elif selector.by_time_period():
+            ts_start, ts_end = selector.time_period
+            query += (f' WHERE (session_start_unix >= {ts_start} AND '
+                      f'session_start_unix < {ts_end})')
 
         getprofiler().enter_region('sqlite session query')
         with self._db_connect(self._db_file()) as conn:
@@ -330,7 +335,8 @@ def _fetch_testcases_from_session(self, selector,
             return []
 
         sessions = self._decode_sessions(
-            results, selector.value if selector.by_session_filter() else None
+            results,
+            selector.sess_filter if selector.by_session_filter() else None
         )
         return [tc for sess in sessions.values()
                 for run in sess['runs'] for tc in run['testcases']
@@ -341,7 +347,7 @@ def _fetch_testcases_from_session(self, selector,
     def _fetch_testcases_time_period(self, ts_start, ts_end, name_patt=None,
                                      test_filter=None):
         expr = (f'job_completion_time_unix >= {ts_start} AND '
-                f'job_completion_time_unix <= {ts_end}')
+                f'job_completion_time_unix < {ts_end}')
         if name_patt:
             expr += f' AND name REGEXP "{name_patt}"'
 
@@ -354,31 +360,34 @@ def _fetch_testcases_time_period(self, ts_start, ts_end, name_patt=None,
     @time_function
     def fetch_testcases(self, selector: QuerySelector,
                         name_patt=None, test_filter=None):
-        if selector.by_time_period():
-            return self._fetch_testcases_time_period(
-                *selector.value, name_patt, test_filter
-            )
-        else:
+        if selector.by_session():
             return self._fetch_testcases_from_session(
                 selector, name_patt, test_filter
             )
+        else:
+            return self._fetch_testcases_time_period(
+                *selector.time_period, name_patt, test_filter
+            )
 
     @time_function
     def fetch_sessions(self, selector: QuerySelector):
         query = 'SELECT uuid, json_blob FROM sessions'
         if selector.by_time_period():
-            ts_start, ts_end = selector.value
+            ts_start, ts_end = selector.time_period
             query += (f' WHERE (session_start_unix >= {ts_start} AND '
-                      f'session_start_unix <= {ts_end})')
+                      f'session_start_unix < {ts_end})')
         elif selector.by_session_uuid():
-            query += f' WHERE uuid == "{selector.value}"'
+            query += f' WHERE uuid == "{selector.uuid}"'
 
+        getprofiler().enter_region('sqlite session query')
         with self._db_connect(self._db_file()) as conn:
             getlogger().debug(query)
             results = conn.execute(query).fetchall()
 
+        getprofiler().exit_region()
         session = self._decode_sessions(
-            results, selector.value if selector.by_session_filter() else None
+            results,
+            selector.sess_filter if selector.by_session_filter() else None
         )
         return [*session.values()]
 
@@ -414,7 +423,7 @@ def _do_remove2(self, conn, uuids):
     @time_function
     def remove_sessions(self, selector: QuerySelector):
         if selector.by_session_uuid():
-            uuids = [selector.value]
+            uuids = [selector.uuid]
         else:
             uuids = [sess['session_info']['uuid']
                      for sess in self.fetch_sessions(selector)]
diff --git a/reframe/frontend/reporting/utility.py b/reframe/frontend/reporting/utility.py
index d1a772b7c..544811b0e 100644
--- a/reframe/frontend/reporting/utility.py
+++ b/reframe/frontend/reporting/utility.py
@@ -154,55 +154,49 @@ def is_uuid(s):
 
 
 class QuerySelector:
-    '''A union class for the different session and testcase queries.
+    '''A class for encapsulating the different session and testcase queries.
 
     A session or testcase query can be of one of the following kinds:
 
-    - Query by time period
     - Query by session uuid
+    - Query by time period
     - Query by session filtering expression
+    - Query by session filtering expression and time period
 
     This class holds only a single value that is interpreted differently,
     depending on how it was constructed.
     There are methods to query the actual kind of the held value, so that
     callers can take appropriate action.
     '''
-    BY_SESS_FILTER = 1
-    BY_SESS_UUID = 2
-    BY_TIME_PERIOD = 3
 
-    def __init__(self, value, kind):
-        self.__value = value
-        self.__kind = kind
+    def __init__(self, *, uuid=None, time_period=None, sess_filter=None):
+        self.__uuid = uuid
+        self.__time_period = time_period
+        self.__sess_filter = sess_filter
+
+    @property
+    def uuid(self):
+        return self.__uuid
 
     @property
-    def value(self):
-        return self.__value
+    def time_period(self):
+        return self.__time_period
 
     @property
-    def kind(self):
-        return self.__kind
+    def sess_filter(self):
+        return self.__sess_filter
 
     def by_time_period(self):
-        return self.__kind == self.BY_TIME_PERIOD
+        return self.__time_period is not None
+
+    def by_session(self):
+        return self.by_session_filter() or self.by_session_uuid()
 
     def by_session_uuid(self):
-        return self.__kind == self.BY_SESS_UUID
+        return self.__uuid is not None
 
     def by_session_filter(self):
-        return self.__kind == self.BY_SESS_FILTER
-
-    @classmethod
-    def from_time_period(cls, ts_start, ts_end):
-        return cls((ts_start, ts_end), cls.BY_TIME_PERIOD)
-
-    @classmethod
-    def from_session_uuid(cls, uuid):
-        return cls(uuid, cls.BY_SESS_UUID)
-
-    @classmethod
-    def from_session_filter(cls, sess_filter):
-        return cls(sess_filter, cls.BY_SESS_FILTER)
+        return self.__sess_filter is not None
 
     def __repr__(self):
         clsname = type(self).__name__
@@ -249,12 +243,17 @@ def parse_query_spec(s):
         return None
 
     if is_uuid(s):
-        return QuerySelector.from_session_uuid(s)
+        return QuerySelector(uuid=s)
 
-    if s.startswith('?'):
-        return QuerySelector.from_session_filter(s[1:])
+    if '?' in s:
+        time_period, sess_filter = s.split('?', maxsplit=1)
+        if time_period:
+            return QuerySelector(sess_filter=sess_filter,
+                                 time_period=parse_time_period(time_period))
+        else:
+            return QuerySelector(sess_filter=sess_filter)
 
-    return QuerySelector.from_time_period(*parse_time_period(s))
+    return QuerySelector(time_period=parse_time_period(s))
 
 
 _Match = namedtuple('_Match',


Test patch:
diff --git a/unittests/test_reporting.py b/unittests/test_reporting.py
index 073356aea..c83a35ca6 100644
--- a/unittests/test_reporting.py
+++ b/unittests/test_reporting.py
@@ -213,7 +213,7 @@ def test_parse_cmp_spec_period(time_period):
     match = parse_cmp_spec(f'{spec}/{spec}/mean:/')
     for query in ('base', 'target'):
         assert getattr(match, query).by_time_period()
-        ts_start, ts_end = getattr(match, query).value
+        ts_start, ts_end = getattr(match, query).time_period
         if 'now' in spec:
             # Truncate splits of seconds if using `now` timestamps
             ts_start = int(ts_start)
@@ -341,15 +341,16 @@ def _uuids(s):
     match = parse_cmp_spec(uuid_spec)
     base_uuid, target_uuid = _uuids(uuid_spec)
     if match.base.by_session_uuid():
-        assert match.base.value == base_uuid
+        assert match.base.uuid == base_uuid
 
     if match.target.by_session_uuid():
-        assert match.target.value == target_uuid
+        assert match.target.uuid == target_uuid
 
 
 @pytest.fixture(params=[
     '?xyz == "123"/?xyz == "789"/mean:/',
-    '?xyz == "789"/mean:/'
+    '?xyz == "789"/mean:/',
+    'now-1d:now?xyz == "789"/mean:/'
 ])
 def sess_filter(request):
     return request.param
@@ -359,10 +360,15 @@ def test_parse_cmp_spec_with_filter(sess_filter):
     match = parse_cmp_spec(sess_filter)
     if match.base:
         assert match.base.by_session_filter()
-        assert match.base.value == 'xyz == "123"'
+        assert match.base.sess_filter == 'xyz == "123"'
 
     assert match.target.by_session_filter()
-    assert match.target.value == 'xyz == "789"'
+    assert match.target.sess_filter == 'xyz == "789"'
+
+    if sess_filter.startswith('now'):
+        assert match.target.by_time_period()
+        ts_start, ts_end = match.target.time_period
+        assert int(ts_end - ts_start) == 86400
 
 
 @pytest.fixture(params=['2024:07:01T12:34:56', '20240701', '20240701:',
@@ -439,6 +445,15 @@ def _count_failed(testcases):
 
         return count
 
+    def from_time_period(ts_start, ts_end):
+        return QuerySelector(time_period=(ts_start, ts_end))
+
+    def from_session_uuid(x):
+        return QuerySelector(uuid=x)
+
+    def from_session_filter(filt, ts_start, ts_end):
+        return QuerySelector(time_period=(ts_start, ts_end), sess_filter=filt)
+
     monkeypatch.setenv('HOME', str(tmp_path))
     uuids = []
     timestamps = []
@@ -451,18 +466,20 @@ def _count_failed(testcases):
         report = _generate_runreport(runner.stats, *tm.timestamps())
         uuids.append(report.store())
 
-    backend = report_storage.StorageBackend.default()
-
-    from_time_period = QuerySelector.from_time_period
-    from_session_uuid = QuerySelector.from_session_uuid
-
     # Test `fetch_sessions`: time period version
+    backend = report_storage.StorageBackend.default()
     now = time.time()
     stored_sessions = backend.fetch_sessions(from_time_period(0, now))
     assert len(stored_sessions) == 2
     for i, sess in enumerate(stored_sessions):
         assert sess['session_info']['uuid'] == uuids[i]
 
+    # Test `fetch_sessions`: session filter version
+    stored_sessions = backend.fetch_sessions(
+        from_session_filter('num_failures==5', timestamps[1][0], now)
+    )
+    assert len(stored_sessions) == 1
+
     # Test `fetch_session`: session uuid version
     for uuid in uuids:
         stored_sessions = backend.fetch_sessions(from_session_uuid(uuid))
@@ -494,6 +511,13 @@ def _count_failed(testcases):
     assert backend.fetch_testcases(from_time_period(timestamps[1][1],
                                                     timestamps[0][0])) == []
 
+    # Test `fetch_testcases`: session filter version
+    testcases = backend.fetch_testcases(
+        from_session_filter('num_failures==5', timestamps[1][0], now),
+        '^HelloTest'
+    )
+    assert len(testcases) == 1
+
     # Test `fetch_testcases`: session version
     for i, uuid in enumerate(uuids):
         testcases = backend.fetch_testcases(from_session_uuid(uuid))