cloud_storage: throw on out of range timequery with spillover

When reading from tiered storage, we create a
`async_manifest_view_cursor` using a query (offset or timestamp) and a
begin offset which is set to the start of the stm region or start of
archive (spillover).

There is a bug inside `async_manifest_view_cursor` which causes it to
throw out_of_range error when spillover contains data which is logically
prefix truncated but matches the timequery. This is mainly because the
begin offset is not properly propagated together with the query which
makes it possible for the query to match a spillover manifest which is
below the begin offset.

In this commit, we remove the logic to ignore the out of range error and
propagate it to the caller.

In a later commit, we will revisit the code so that this edge cases is
handled correctly inside the async_manifest_view and it does seek to the
correct offset rather than throwing an out of range exception up to the
client.

src/v/cloud_storage/remote_partition.cc

@@ -20,6 +20,7 @@
 #include "cloud_storage/tx_range_manifest.h"
 #include "cloud_storage/types.h"
 #include "model/fundamental.h"
+#include "model/timestamp.h"
 #include "net/connection.h"
 #include "ssx/future-util.h"
 #include "ssx/watchdog.h"
@@ -605,27 +606,23 @@ class partition_record_batch_reader_impl final
                 [&](model::timestamp query_ts) {
                     // Special case, it can happen when a timequery falls below
                     // the clean offset. Caused when the query races with
-                    // retention/gc. log a warning, since the kafka client can
-                    // handle a failed query
+                    // retention/gc.
                     auto const& spillovers = _partition->_manifest_view
                                                ->stm_manifest()
                                                .get_spillover_map();
-                    if (
-                      spillovers.empty()
-                      || spillovers.get_max_timestamp_column()
-                             .last_value()
-                             .value_or(model::timestamp::max()())
-                           >= query_ts()) {
+
+                    bool timestamp_inside_spillover
+                      = query_ts() <= spillovers.get_max_timestamp_column()
+                                        .last_value()
+                                        .value_or(model::timestamp::min()());
+
+                    if (timestamp_inside_spillover) {
                         vlog(
                           _ctxlog.debug,
                           "Manifest query raced with retention and the result "
                           "is below the clean/start offset for {}",
                           query_ts);
-                        return true;
                     }
-
-                    // query was not meant for archive region. fallthrough and
-                    // log an error
                     return false;
                 })) {
                 // error was handled

src/v/cloud_storage/tests/remote_partition_test.cc

@@ -2417,9 +2417,17 @@ FIXTURE_TEST(test_out_of_range_spillover_query, cloud_storage_fixture) {
           return what.find("Failed to query spillover manifests") != what.npos;
       });
 
-    // BUG: This assertion is disabled because it currently fails.
-    // test_log.debug("Timestamp undershoots the partition");
-    // BOOST_TEST_REQUIRE(timequery(*this, model::timestamp(100), 3 * 6));
+    // Timestamp undershoots the partition.
+    // It shouldn't throw. But as an interim workaround we throw until
+    // clean offset catches up with start offset.
+    // The clients will retry.
+    BOOST_REQUIRE_EXCEPTION(
+      timequery(*this, model::timestamp(100), 3 * 6),
+      std::runtime_error,
+      [](const auto& ex) {
+          ss::sstring what{ex.what()};
+          return what.find("Failed to query spillover manifests") != what.npos;
+      });
 
     test_log.debug("Timestamp within valid spillover but below archive start");
     BOOST_TEST_REQUIRE(
@@ -2435,4 +2443,22 @@ FIXTURE_TEST(test_out_of_range_spillover_query, cloud_storage_fixture) {
 
     test_log.debug("Timestamp overshoots the partition");
     BOOST_TEST_REQUIRE(timequery(*this, model::timestamp::max(), 0));
+
+    // Rewrite the manifest with clean offset to match start offset.
+    manifest.set_archive_clean_offset(
+      archive_start, manifest.archive_size_bytes() / 2);
+    vlog(
+      test_util_log.info,
+      "Rewriting manifest at {}:\n{}",
+      manifest.get_manifest_path(),
+      ostr.str());
+
+    remove_expectations({manifest_url});
+    add_expectations({
+      cloud_storage_fixture::expectation{
+        .url = manifest_url, .body = serialize_manifest(manifest)},
+    });
+
+    // This query should now succeed.
+    BOOST_TEST_REQUIRE(timequery(*this, model::timestamp(100), 3 * 6));
 }