Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

db: paginated sequence for temporal KV API #2186

Merged
merged 7 commits into from
Jul 21, 2024

Conversation

canepat
Copy link
Member

@canepat canepat commented Jul 18, 2024

This PR introduces support for a hand-crafted paginated sequence of values to be used in the definition of temporal KV API endpoints. Such abstraction allows user code to navigate a paginated contiguous sequence of values without bothering about pagination at call site.

If performance measurements in standalone mode (i.e. direct KV client) will show significant overhead caused by this approach, we can relax this requirement to hide pagination at call site and try to obtain faster code.

Design Alternatives
We're going to use this abstraction for both direct and gRPC KV client implementations and we cannot block the coroutine scheduler in gRPC case, so we do need a common asynchronous interface definition. This requirement (which could be challenged if performance results are not satisfactory) excludes usage of a C++ iterator.
Hence, I have considered the following alternative options:

  • C++23 std::generator reference implementation: not feasible because it does not allow usage of co_await within the generator coroutine
  • asynchronous generator based on C++20 coroutines: some examples exists here and here, which however do not play nicely with Asio. Another feasible option could be cobalt implementation, which is based on and is compatible with Asio, but it would require to add a new dependency and using other companion cobalt abstractions just for one class: this seems too much.
  • experimental Asio coro abstraction acting as task/generator/awaitable: this looks promising but it's still experimental so there are substantial issues and again would require a large refactoring in our codebase.

Notes
Evaluating usage of Cobalt could be interesting as a research sub-project, so I've opened #2185

@canepat canepat added enhancement New feature or improvement temporal kv Related to Temporal KV internal gRPC interface labels Jul 18, 2024
web-flow and others added 2 commits July 18, 2024 23:08
fix gcc error insufficient contextual information to determine type
Copy link

codecov bot commented Jul 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 53.52%. Comparing base (cdac1b3) to head (07fc51a).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2186      +/-   ##
==========================================
- Coverage   53.52%   53.52%   -0.01%     
==========================================
  Files         687      687              
  Lines       47834    47834              
  Branches     7253     7253              
==========================================
- Hits        25602    25601       -1     
- Misses      20003    20005       +2     
+ Partials     2229     2228       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@canepat canepat marked this pull request as ready for review July 19, 2024 16:08
@canepat canepat merged commit 5756523 into master Jul 21, 2024
15 checks passed
@canepat canepat deleted the ci/db_kv_api_paginated_sequence branch July 21, 2024 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement temporal kv Related to Temporal KV internal gRPC interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants