Skip to content

Commit 1e9fbf9

Browse files
lsulakCopilot
andcommitted
fix: stack-safe paging of all checkpoints and ADR 02 response shape
queryAllPages recursed through flatMap, which for strict effects (e.g. Identity) runs the continuation right away, adding stack frames per page. The continuation now hands the collected records over to a tail-recursive loop when it runs synchronously, and recurses within the effect otherwise (asynchronous or lazy effects), the hand-over being atomic. ADR 02: a flow checkpoints page item is one checkpoint with its measurements nested (processStartTime, measurements, partitioning), and parent -> child edges cannot be reconstructed today (Bet 3). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d2814f0 commit 1e9fbf9

5 files changed

Lines changed: 135 additions & 14 deletions

File tree

‎adrs/02-consumption-pattern-for-audits.md‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ Three steps, all on **today's** v2 API:
8080
`PATCH .../ancestors` joins the new parent's flows, but its already existing descendants do not.)
8181
3. **Pull all measurements across the chain, newest first, paged.**
8282
`GET /api/v2/flows/{flowId}/checkpoints?limit=100&offset=0&include-properties=true`.
83-
Each row = one measure of one checkpoint, **tagged with its partitioning**. That is the audit dataset:
84-
measurements + their dataset relationships across the whole lineage chain.
83+
Each item = one checkpoint with all its measurements nested, **tagged with its partitioning**. That is the audit
84+
dataset: measurements + their dataset relationships across the whole lineage chain.
8585

8686
### Worked example (HTTP)
8787

@@ -96,16 +96,16 @@ GET /api/v2/partitionings/2000000010448528/main-flow ->
9696
GET /api/v2/flows/3000000000123456/checkpoints?limit=100&offset=0&include-properties=true
9797
```
9898

99-
Each page item carries `checkpointStartTime`, `measureName`/`measurementValue`, and the `partitioning` it belongs
100-
to — so relationships come for free.
99+
Each page item is one checkpoint carrying its `processStartTime`, its `measurements` (each a `measure` - name and
100+
measured columns - with its `result`), and the `partitioning` it belongs to — so relationships come for free.
101101

102102
### "Last 2 months" today (no server change needed)
103103

104-
Because results are latest-first, page step 3 and **stop at the first row older than the cutoff**:
104+
Because results are latest-first, page step 3 and **stop at the first checkpoint older than the cutoff**:
105105

106106
```
107-
keep the page while checkpointStartTime >= now - 2 months
108-
stop as soon as a row is older (all later rows are older too)
107+
keep the page while processStartTime >= now - 2 months
108+
stop as soon as a checkpoint is older (all later checkpoints are older too)
109109
```
110110

111111
This works on the current API. Its only cost is over-fetching whole pages and re-implementing the cutoff in every
@@ -318,7 +318,8 @@ are deferred.)*
318318
time). For late/backfilled reports the two can differ.
319319
- **Volume at scale.** A long-lived flow accumulates many checkpoints; the proposed index is what keeps a
320320
time-window audit query cheap.
321-
- **Explicit parent -> child edges.** Flow membership gives the downstream lineage *set*; if an audit needs the exact
322-
edge list (who is parent of whom), that is reconstructed today via `get_partitioning_ancestors` per node. A
323-
dedicated "flow graph" (nodes + edges) endpoint is a possible small future addition.
321+
- **Explicit parent -> child edges.** Flow membership gives the downstream lineage *set* only. If an audit needs the
322+
exact edge list (who is parent of whom), it cannot be reconstructed today: `get_partitioning_ancestors` returns the
323+
flow roots a partitioning belongs to, not its immediate parents (see §6). The edges have to be persisted first
324+
(Bet 3); a dedicated "flow graph" (nodes + edges) endpoint can follow.
324325
- **Access control** for audit consumers (read-only scopes/authorization) is out of scope here.

‎reader/src/main/scala/za/co/absa/atum/reader/core/Reader.scala‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,14 @@ import sttp.model.Uri
2323
import sttp.monad.MonadError
2424
import sttp.monad.syntax._
2525
import za.co.absa.atum.model.envelopes.SuccessResponse.PaginatedResponse
26+
import za.co.absa.atum.reader.core.Reader.{Detached, NextPage, PageHandOver, Pending}
2627
import za.co.absa.atum.reader.core.RequestResult._
2728
import za.co.absa.atum.reader.server.ServerConfig
2829
import za.co.absa.atum.reader.exceptions.RequestException.CirceError
2930

31+
import java.util.concurrent.atomic.AtomicReference
32+
import scala.annotation.tailrec
33+
3034
/**
3135
* Reader is a base class for reading data from a remote server.
3236
* @param serverConfig - the configuration how to reach the Atum server
@@ -52,6 +56,9 @@ abstract class Reader[F[_]](implicit
5256
/**
5357
* Queries the pages one after another, starting at offset 0, until the server reports there is no more data.
5458
*
59+
* The paging is stack-safe for any effect, including the strict ones (e.g. `Identity`), where `flatMap` runs its
60+
* continuation right away and a plain recursion would add stack frames with each page.
61+
*
5562
* @param pageSize - the size of the page (record count) to query
5663
* @param queryPage - function querying a page of the given size (limit) at the given offset
5764
* @return - the records of all the pages, in order, or the first error encountered
@@ -60,13 +67,33 @@ abstract class Reader[F[_]](implicit
6067
pageSize: Int,
6168
queryPage: (Int, Long) => F[RequestResult[PaginatedResponse[T]]]
6269
): F[RequestResult[Seq[T]]] = {
70+
// If the continuation of a page runs before `flatMap` returns (strict effect), it only hands the records collected
71+
// so far over to the loop, which then queries the next page iteratively. Otherwise (asynchronous or lazy effect)
72+
// the continuation queries the next page itself, the effect taking care of the stack. The hand-over is atomic, as
73+
// the continuation of an asynchronous effect may run on another thread at the same time.
74+
@tailrec
6375
def queryFrom(offset: Long, collected: Vector[T]): F[RequestResult[Seq[T]]] = {
64-
queryPage(pageSize, offset).flatMap {
65-
case Right(page) if page.pagination.hasMore => queryFrom(offset + pageSize, collected ++ page.data)
76+
val handOver = new AtomicReference[PageHandOver[T]](Pending)
77+
val result = queryPage(pageSize, offset).flatMap {
78+
case Right(page) if page.pagination.hasMore =>
79+
val nextCollected = collected ++ page.data
80+
if (handOver.compareAndSet(Pending, new NextPage(nextCollected))) {
81+
me.unit(RequestOK[Seq[T]](nextCollected)) // discarded, the loop continues with the next page
82+
} else {
83+
queryFromWithinEffect(offset + pageSize, nextCollected)
84+
}
6685
case Right(page) => me.unit(RequestOK[Seq[T]](collected ++ page.data))
6786
case Left(error) => me.unit(RequestFail[Seq[T]](error))
6887
}
88+
handOver.getAndSet(Detached) match {
89+
case nextPage: NextPage[T] => queryFrom(offset + pageSize, nextPage.collected)
90+
case _ => result
91+
}
6992
}
93+
// not a tail call, therefore kept apart for `queryFrom` to remain tail-recursive
94+
def queryFromWithinEffect(offset: Long, collected: Vector[T]): F[RequestResult[Seq[T]]] =
95+
queryFrom(offset, collected)
96+
7097
queryFrom(0, Vector.empty)
7198
}
7299

@@ -85,3 +112,10 @@ abstract class Reader[F[_]](implicit
85112
response.map(_.toRequestResult)
86113
}
87114
}
115+
116+
object Reader {
117+
private sealed class PageHandOver[+T]
118+
private final class NextPage[+T](val collected: Vector[T]) extends PageHandOver[T]
119+
private val Pending: PageHandOver[Nothing] = new PageHandOver
120+
private val Detached: PageHandOver[Nothing] = new PageHandOver
121+
}

‎reader/src/test/scala/za/co/absa/atum/reader/core/ReaderUnitTests.scala‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import sttp.client3.testing.SttpBackendStub
2222
import sttp.client3.{Identity, SttpBackend}
2323
import sttp.client3.monad.IdMonad
2424
import sttp.monad.MonadError
25+
import za.co.absa.atum.model.envelopes.Pagination
26+
import za.co.absa.atum.model.envelopes.SuccessResponse.PaginatedResponse
2527
import za.co.absa.atum.reader.core.RequestResult.{RequestFail, RequestOK, RequestResult}
2628
import za.co.absa.atum.reader.exceptions.RequestException.ParsingException
2729
import za.co.absa.atum.reader.server.ServerConfig
@@ -36,6 +38,13 @@ class ReaderUnitTests extends AnyFunSuiteLike {
3638
override def mapRequestResultF[I, O](requestResult: RequestResult[I], f: I => F[RequestResult[O]]): F[RequestResult[O]] = {
3739
super.mapRequestResultF(requestResult, f)
3840
}
41+
42+
override def queryAllPages[T](
43+
pageSize: Int,
44+
queryPage: (Int, Long) => F[RequestResult[PaginatedResponse[T]]]
45+
): F[RequestResult[Seq[T]]] = {
46+
super.queryAllPages(pageSize, queryPage)
47+
}
3948
}
4049

4150
private implicit val serverConfig: ServerConfig = ServerConfig("http://localhost:8080")
@@ -57,4 +66,22 @@ class ReaderUnitTests extends AnyFunSuiteLike {
5766
val result = reader.mapRequestResultF(requestResult, fnc)
5867
assert(result == requestResult)
5968
}
69+
70+
test("All pages are queried, without exhausting the stack even when the effect is synchronous") {
71+
val recordCount = 100000L
72+
def queryPage(limit: Int, offset: Long): Identity[RequestResult[PaginatedResponse[Long]]] =
73+
RequestOK(PaginatedResponse(Seq(offset), Pagination(limit, offset, hasMore = offset + limit < recordCount)))
74+
val reader = new ReaderForTesting[Identity]
75+
val result = reader.queryAllPages(1, queryPage)
76+
assert(result == RequestOK(0L until recordCount))
77+
}
78+
79+
test("Querying all pages stops at the first page failing") {
80+
def queryPage(limit: Int, offset: Long): Identity[RequestResult[PaginatedResponse[Long]]] =
81+
if (offset < 2) RequestOK(PaginatedResponse(Seq(offset), Pagination(limit, offset, hasMore = true)))
82+
else RequestFail(ParsingException("Just a test", ""))
83+
val reader = new ReaderForTesting[Identity]
84+
val result = reader.queryAllPages(1, queryPage)
85+
assert(result == RequestFail(ParsingException("Just a test", "")))
86+
}
6087
}

‎reader/src/test/scala/za/co/absa/atum/reader/core/Reader_CatsIOUnitTests.scala‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ import sttp.client3.SttpBackend
2323
import sttp.client3.testing.SttpBackendStub
2424
import sttp.monad.{MonadAsyncError, MonadError}
2525
import za.co.absa.atum.model.dto.PartitionDTO
26+
import za.co.absa.atum.model.envelopes.Pagination
27+
import za.co.absa.atum.model.envelopes.SuccessResponse.PaginatedResponse
2628
import za.co.absa.atum.model.utils.JsonSyntaxExtensions.JsonSerializationSyntax
27-
import za.co.absa.atum.reader.core.RequestResult.RequestResult
29+
import za.co.absa.atum.reader.core.RequestResult.{RequestOK, RequestResult}
2830
import za.co.absa.atum.reader.server.ServerConfig
2931

3032
class Reader_CatsIOUnitTests extends AnyFunSuiteLike {
@@ -33,6 +35,10 @@ class Reader_CatsIOUnitTests extends AnyFunSuiteLike {
3335
private class ReaderForTest[F[_]](implicit serverConfig: ServerConfig, backend: SttpBackend[F, Any], ev: MonadError[F])
3436
extends Reader {
3537
override def getQuery[R: Decoder](endpointUri: String, params: Map[String, String]): F[RequestResult[R]] = super.getQuery(endpointUri, params)
38+
override def queryAllPages[T](
39+
pageSize: Int,
40+
queryPage: (Int, Long) => F[RequestResult[PaginatedResponse[T]]]
41+
): F[RequestResult[Seq[T]]] = super.queryAllPages(pageSize, queryPage)
3642
}
3743

3844
test("Using Cats IO based backend") {
@@ -49,4 +55,19 @@ class Reader_CatsIOUnitTests extends AnyFunSuiteLike {
4955
assert(result == Right(partitionDTO))
5056
}
5157

58+
test("Querying all pages using Cats IO based backend, the query being repeatable") {
59+
import cats.effect.IO
60+
import za.co.absa.atum.reader.implicits.io.catsIOMonadError
61+
62+
implicit val server: SttpBackendStub[IO, Any] = SttpBackendStub[IO, Any](implicitly[MonadAsyncError[IO]])
63+
val recordCount = 10000L
64+
def queryPage(limit: Int, offset: Long): IO[RequestResult[PaginatedResponse[Long]]] =
65+
IO(RequestOK(PaginatedResponse(Seq(offset), Pagination(limit, offset, hasMore = offset + limit < recordCount))))
66+
67+
val reader = new ReaderForTest
68+
val query = reader.queryAllPages(1, queryPage)
69+
assert(query.unsafeRunSync() == RequestOK(0L until recordCount))
70+
assert(query.unsafeRunSync() == RequestOK(0L until recordCount))
71+
}
72+
5273
}

‎reader/src/test/scala/za/co/absa/atum/reader/core/Reader_FutureUnitTests.scala‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,14 @@ import sttp.client3.SttpBackend
2222
import sttp.client3.testing.SttpBackendStub
2323
import sttp.monad.MonadError
2424
import za.co.absa.atum.model.dto.PartitionDTO
25+
import za.co.absa.atum.model.envelopes.Pagination
26+
import za.co.absa.atum.model.envelopes.SuccessResponse.PaginatedResponse
2527
import za.co.absa.atum.model.utils.JsonSyntaxExtensions.JsonSerializationSyntax
26-
import za.co.absa.atum.reader.core.RequestResult.RequestResult
28+
import za.co.absa.atum.reader.core.RequestResult.{RequestFail, RequestOK, RequestResult}
29+
import za.co.absa.atum.reader.exceptions.RequestException.ParsingException
2730
import za.co.absa.atum.reader.server.ServerConfig
2831

32+
import scala.concurrent.ExecutionContext.Implicits.global
2933
import scala.concurrent.duration.Duration
3034
import scala.concurrent.{Await, Future}
3135

@@ -35,6 +39,10 @@ class Reader_FutureUnitTests extends AnyFunSuiteLike {
3539
private class ReaderForTest[F[_]](implicit serverConfig: ServerConfig, backend: SttpBackend[F, Any], ev: MonadError[F])
3640
extends Reader {
3741
override def getQuery[R: Decoder](endpointUri: String, params: Map[String, String]): F[RequestResult[R]] = super.getQuery(endpointUri, params)
42+
override def queryAllPages[T](
43+
pageSize: Int,
44+
queryPage: (Int, Long) => F[RequestResult[PaginatedResponse[T]]]
45+
): F[RequestResult[Seq[T]]] = super.queryAllPages(pageSize, queryPage)
3846
}
3947

4048
test("Using Future based backend") {
@@ -50,4 +58,34 @@ class Reader_FutureUnitTests extends AnyFunSuiteLike {
5058
assert(result == Right(partitionDTO))
5159
}
5260

61+
test("Querying all pages using Future based backend") {
62+
import za.co.absa.atum.reader.implicits.future.futureMonadError
63+
64+
implicit val server: SttpBackend[Future, Any] = SttpBackendStub.asynchronousFuture
65+
val recordCount = 1000L
66+
// alternating already completed and asynchronously completing pages
67+
def queryPage(limit: Int, offset: Long): Future[RequestResult[PaginatedResponse[Long]]] = {
68+
def page = RequestOK(PaginatedResponse(Seq(offset), Pagination(limit, offset, hasMore = offset + limit < recordCount)))
69+
if (offset % 2 == 0) Future.successful(page) else Future(page)
70+
}
71+
72+
val reader = new ReaderForTest
73+
val result = Await.result(reader.queryAllPages(1, queryPage), Duration(10, "second"))
74+
assert(result == RequestOK(0L until recordCount))
75+
}
76+
77+
test("Querying all pages using Future based backend stops at the first page failing") {
78+
import za.co.absa.atum.reader.implicits.future.futureMonadError
79+
80+
implicit val server: SttpBackend[Future, Any] = SttpBackendStub.asynchronousFuture
81+
def queryPage(limit: Int, offset: Long): Future[RequestResult[PaginatedResponse[Long]]] = Future {
82+
if (offset < 2) RequestOK(PaginatedResponse(Seq(offset), Pagination(limit, offset, hasMore = true)))
83+
else RequestFail(ParsingException("Just a test", ""))
84+
}
85+
86+
val reader = new ReaderForTest
87+
val result = Await.result(reader.queryAllPages(1, queryPage), Duration(3, "second"))
88+
assert(result == RequestFail(ParsingException("Just a test", "")))
89+
}
90+
5391
}

0 commit comments

Comments
 (0)