Skip to content

Declare a top-K aggregation on a group and list what declares one (Steps 1 and 2 of #386) - #493

Merged
zipdoki merged 5 commits into
mainfrom
feat/topk-metadata
Aug 27, 2026
Merged

zipdoki merged 5 commits into
mainfrom
feat/topk-metadata

Conversation

@zipdoki

@zipdoki zipdoki commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Introduce top-K aggregation metadata on Group and expose a read endpoint listing tables that declare one.

A Group can now carry an Aggregations block. Today only topk is defined; the schema is designed so future aggregation kinds (e.g. HLL) can land as sibling fields without restructuring the payload.

GET /graph/v3/aggregations returns a flat list of (type, database, table) tuples for every table that declares an aggregation, filterable by ?type=. The aggregate/mutate flow that consumes this metadata is out of scope for this PR and will land separately.

Metadata example

A group inside a label creation payload:

{
  "groups": [
    {
      "group": "purchased_count",
      "type": "COUNT",
      "fields": [{"name": "_target"}],
      "directionType": "OUT",
      "aggregations": {
        "topk": [
          {
            "topk": "top_purchased",
            "entity": "_source",
            "ranges": "_target:eq:{_target}",
            "dimension": "_target",
            "refreshAfterMillis": 31536000000,
            "rank": "commerce.purchases__topk"
          }
        ]
      }
    }
  ]
}

Endpoint example

GET /graph/v3/aggregations

{
  "items": [
    { "type": "TOPK", "database": "commerce", "table": "purchases" }
  ]
}

GET /graph/v3/aggregations?type=TOPK filters to a single aggregation type.

Test plan

  • ./gradlew :core:test --tests 'com.kakao.actionbase.core.metadata.common.AggregationsTest'
  • ./gradlew :engine:test --tests 'com.kakao.actionbase.engine.service.AggregationServiceSpec'
  • ./gradlew :engine:test --tests 'com.kakao.actionbase.v2.engine.entity.LabelEntityFunctionsSpec'
  • ./gradlew :server:test --tests 'com.kakao.actionbase.server.api.graph.v3.metadata.MetadataAggControllerE2ETest'

AI Assistance

  • This PR was written largely with AI assistance.
    • Tool / model: Claude Code (Opus 4.7)

@zipdoki zipdoki self-assigned this Aug 19, 2026
@zipdoki
zipdoki requested a review from em3s as a code owner August 19, 2026 04:13
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Aug 19, 2026
The refresh queue is a constant, so every ranking in the system shares one queue and one sweeper. Naming the
queue per declaration gives a set of rankings a sweeper of its own, on its own schedule and with its own
backlog, and lets a stuck ranking family be swept without holding up the rest.

`refreshQueue` defaults to the queue everything already used, so a declaration written before this keeps
refreshing where it always did. Nothing reads the field yet -- the enqueue lives on `feat/topk-aggregate`.
zipdoki added a commit that referenced this pull request Aug 27, 2026
`refreshQueue` landed on the declaration in #493 with nothing reading it. The enqueue still went to the
constant, so naming a queue had no effect.

Existing tests here name no queue and assert the shared one, which pins the default; the new test names one
and also asserts nothing reaches the shared queue.
@zipdoki

zipdoki commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Optimistic Merge

@zipdoki
zipdoki merged commit c58b778 into main Aug 27, 2026
3 checks passed
@zipdoki
zipdoki deleted the feat/topk-metadata branch August 27, 2026 08:08
zipdoki added a commit that referenced this pull request Aug 27, 2026
The branch was rebased onto the squash-merged #493, where the constant is `REFRESH_QUEUE`. The rebase carried that name into `AggregationConstants`, `Group` and `EdgeSchemaSerializationTest` but left `TopkAggregationHandlerTest` and `MetadataAggControllerE2ETest` on `REFRESH_TABLE`, so the branch did not compile. Point them at the merged name.
zipdoki added a commit that referenced this pull request Aug 27, 2026
)

* Run a top-K aggregation and write the ranked rows

* Pin how a date bucket rounds a value and a range bound

* Schedule a top-K refresh for the instant its bucket leaves the window

* Read a bucket format's precision in one place

* Escape `|` in a rank key so two rankings never share a row

* Schedule a refresh from the bucketed field rather than from the bucket's name

A declaration may name a date bucket for the value it yields rather than for the field it reads, and a
`ranges` bound refers to it by that name. `refreshAt` looked the event's value up under the bucket's name,
found nothing whenever the two names differ, and fell back to counting from now — so an order that left
the window a year ago was scheduled to expire a year from ingest instead.

Nothing reported it. The aggregation answered SUCCESS and the message carried the wrong due time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Enqueue a refresh onto the queue the declaration named

`refreshQueue` landed on the declaration in #493 with nothing reading it. The enqueue still went to the
constant, so naming a queue had no effect.

Existing tests here name no queue and assert the shared one, which pins the default; the new test names one
and also asserts nothing reaches the shared queue.

* Aggregation results come back in request order

`AggregationResult` carries no field naming the item it came from, so a caller pairs a result back to its request by position. The dispatcher merged handler output as it completed, which left the same batch coming back ordered by whichever writes finished first — a three-item batch returns fully reversed when the first item is the slowest.

`Graph.mutate` and `AbstractLabel.mutate` already made this choice on the mutation path. Concurrency is unchanged: `flatMapSequential` and `mergeSequential` subscribe eagerly under the same default `maxConcurrency`, so storage sees the same request pattern.

`AggregationService.aggregate` had no test of its own. The new ones pin fan-out, the empty batch, request ordering, and the fact that two handlers claiming one type collapse to the last — that last one is current behavior, pinned rather than fixed.

* Top-K tests name the refresh queue constant main declares

The branch was rebased onto the squash-merged #493, where the constant is `REFRESH_QUEUE`. The rebase carried that name into `AggregationConstants`, `Group` and `EdgeSchemaSerializationTest` but left `TopkAggregationHandlerTest` and `MetadataAggControllerE2ETest` on `REFRESH_TABLE`, so the branch did not compile. Point them at the merged name.

* Drop the unreachable copy of `parseFqn`

`AggregationFunctions.kt` and `TopkAggregationHandler` both declared `parseFqn`, added in the same commit. The handler's own private copy shadows the top-level one at the only call site, so nothing reached `AggregationFunctions` and the two spellings of its failure message could drift apart unnoticed. Extract a shared one again when a second handler needs it.

* Pin that a window which never moves gets no refresh

`processTopk` skips the refresh on three conditions and only two were covered. The third — a top-K that sets `refreshAfterMillis` but bounds its range with fixed dates — had no test, so nothing said what a non-sliding window is supposed to do. Drop the guard and this new test is the only one that fails, which is what it is for.

Also drops `returns a result for every item`, added earlier today: the ordering test asserts the same three items with `shouldContainExactly`, so it already covers membership.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
zipdoki added a commit that referenced this pull request Aug 31, 2026
…Step 5 of #386) (#497)

* Run a top-K aggregation and write the ranked rows

* Pin how a date bucket rounds a value and a range bound

* Schedule a top-K refresh for the instant its bucket leaves the window

* Read a bucket format's precision in one place

* Escape `|` in a rank key so two rankings never share a row

* Schedule a refresh from the bucketed field rather than from the bucket's name

A declaration may name a date bucket for the value it yields rather than for the field it reads, and a
`ranges` bound refers to it by that name. `refreshAt` looked the event's value up under the bucket's name,
found nothing whenever the two names differ, and fell back to counting from now — so an order that left
the window a year ago was scheduled to expire a year from ingest instead.

Nothing reported it. The aggregation answered SUCCESS and the message carried the wrong due time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Enqueue a refresh onto the queue the declaration named

`refreshQueue` landed on the declaration in #493 with nothing reading it. The enqueue still went to the
constant, so naming a queue had no effect.

Existing tests here name no queue and assert the shared one, which pins the default; the new test names one
and also asserts nothing reaches the shared queue.

* Recompute a ranking from a refresh message

* Remove the expire payloads the refresh queue replaced

The sweep design moved from an _expire table scanned by expiredAt to a
refresh queue, and these three payloads never got wired to anything.
AggregationExpireItemRequest has no references at all, and
AggregationExpireResult is only read by AggregationsExpireResponse.from,
which nothing calls.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Read a materialized top-K by naming its entity and dimension values

* Drop the stray import blank line the merge left in `MetadataAggController`

The auto-merge kept a blank line the branch had between `PostMapping` and
`RequestBody`, splitting the `org.springframework` import group in two.
`:server:spotlessKotlinCheck` fails on it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant