feat(storagenode): add pipelined Append RPC handler by ijsong · Pull Request #457 · kakao/varlog

ijsong · 2023-05-23T09:00:03Z

What this PR does

This patch changes the Append RPC handler to support pipelined requests and does not change the client's API. Therefore, users can use Append API transparently.

Supporting pipelined requests can lead to overhead since it is necessary to have additional goroutines and concurrent queues. To lower additional overhead, this change uses reader-biased mutex instead of built-in RWMutex to avoid shared lock contention. As a result of experimentations, this PR showed very little overhead. Furthermore, we can improve the existing Append API more efficiently using a long-lived stream: the current implementation creates a new stream whenever calling Append API, which leads to unnecessary tasks such as RPC initiation. We can reuse long-lived streams by changing client API. See this issue at #458.

Which issue(s) this PR resolves

This PR implements server-side parts of LogStreamAppender mentioned in #433. It also can be used for pipelining generic Append RPC said in #441.

ijsong · 2023-05-23T09:00:30Z

Current dependencies on/for this PR:

main
- PR ci(lint): fix depguard configuration #463
  - PR feat(storagenode): change append rpc from unary to stream #449
    - PR fix(storagenode): support partial success/failure for append #450
      - PR feat(storagenode): add configurations for initial window size #451
        
        PR feat(storagenode): add pipelined Append RPC handler #457 👈
        
        PR feat(client): add LogStreamAppender #459
        
        PR test(benchmark): support log stream appender #462

This comment was auto-generated by Graphite.

codecov-commenter · 2023-05-24T07:38:09Z

Codecov Report

Patch coverage: 52.38% and project coverage change: -0.03 ⚠️

Comparison is base (82c1e97) 63.18% compared to head (80abf51) 63.15%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@                               Coverage Diff                                @@
##           grpc_server_initial_flow_control_window_size     #457      +/-   ##
================================================================================
- Coverage                                         63.18%   63.15%   -0.03%     
================================================================================
  Files                                               132      132              
  Lines                                             17958    18145     +187     
================================================================================
+ Hits                                              11346    11459     +113     
- Misses                                             6040     6120      +80     
+ Partials                                            572      566       -6

Impacted Files	Coverage Δ
internal/storagenode/logstream/append.go	`47.11% <0.00%> (-32.59%)`	⬇️
internal/storagenode/config.go	`47.40% <12.50%> (-2.20%)`	⬇️
internal/storagenode/log_server.go	`81.46% <90.59%> (+8.67%)`	⬆️
internal/storagenode/storagenode.go	`72.83% <100.00%> (+0.16%)`	⬆️

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

internal/storagenode/log_server.go

Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 3.4.0 to 3.5.0. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](golangci/golangci-lint-action@v3.4.0...v3.5.0) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.8.3 to 1.8.4. - [Release notes](https://github.com/stretchr/testify/releases) - [Commits](stretchr/testify@v1.8.3...v1.8.4) --- updated-dependencies: - dependency-name: github.com/stretchr/testify dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>

…/golangci-lint-action-3.5.0 build(deps): bump golangci/golangci-lint-action from 3.4.0 to 3.5.0

…stify-1.8.4

…tretchr/testify-1.8.4 build(deps): bump github.com/stretchr/testify from 1.8.3 to 1.8.4

Bumps [golang.org/x/tools](https://github.com/golang/tools) from 0.9.1 to 0.9.3. - [Release notes](https://github.com/golang/tools/releases) - [Commits](golang/tools@v0.9.1...v0.9.3) --- updated-dependencies: - dependency-name: golang.org/x/tools dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>

…/tools-0.9.3 build(deps): bump golang.org/x/tools from 0.9.1 to 0.9.3

Bumps [github.com/urfave/cli/v2](https://github.com/urfave/cli) from 2.25.3 to 2.25.5. - [Release notes](https://github.com/urfave/cli/releases) - [Changelog](https://github.com/urfave/cli/blob/main/docs/CHANGELOG.md) - [Commits](urfave/cli@v2.25.3...v2.25.5) --- updated-dependencies: - dependency-name: github.com/urfave/cli/v2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>

…rfave/cli/v2-2.25.5 build(deps): bump github.com/urfave/cli/v2 from 2.25.3 to 2.25.5

ijsong · 2023-06-07T16:30:58Z

@ijsong started a stack merge that includes this pull request via Graphite.

This patch changes the Append RPC handler to support pipelined requests and does not change the client's API. Therefore, users can use Append API transparently. Supporting pipelined requests can lead to overhead since it is necessary to have additional goroutines and concurrent queues. To lower additional overhead, this change uses [reader-biased mutex](https://github.com/puzpuzpuz/xsync#rbmutex) instead of built-in RWMutex to avoid shared lock contention. As a result of experimentations, this PR showed very little overhead. Furthermore, we can improve the existing Append API more efficiently [using a long-lived stream](https://grpc.io/docs/guides/performance/#general): the current implementation creates a new stream whenever calling Append API, which leads to unnecessary tasks such as RPC initiation. We can reuse long-lived streams by changing client API. See this issue at #458. This PR implements server-side parts of LogStreamAppender mentioned in #433. It also can be used for pipelining generic Append RPC said in #441.

ijsong · 2023-06-07T16:31:19Z

Graphite rebased this pull request as part of a merge.

ijsong · 2023-06-07T16:45:32Z

@ijsong merged this pull request with Graphite.

ijsong requested a review from hungryjang as a code owner May 23, 2023 09:00

ijsong mentioned this pull request May 23, 2023

storagenode,client: introduce LogStreamAppender #433

Closed

3 tasks

ijsong self-assigned this May 23, 2023

ijsong marked this pull request as draft May 23, 2023 09:33

ijsong force-pushed the pipelined_append_handler branch 2 times, most recently from 2a8118a to 317743d Compare May 24, 2023 07:22

ijsong marked this pull request as ready for review May 24, 2023 07:41

ijsong force-pushed the pipelined_append_handler branch from 317743d to 80abf51 Compare May 24, 2023 12:05

ijsong marked this pull request as draft May 25, 2023 06:04

ijsong mentioned this pull request May 25, 2023

feat(client): add LogStreamAppender #459

Merged

ijsong linked an issue May 25, 2023 that may be closed by this pull request

storagenode,client: introduce LogStreamAppender #433

Closed

3 tasks

ijsong marked this pull request as ready for review May 25, 2023 06:06

ijsong force-pushed the grpc_server_initial_flow_control_window_size branch from 82c1e97 to 3d0cdbf Compare June 1, 2023 03:32

ijsong force-pushed the pipelined_append_handler branch from 80abf51 to 0579220 Compare June 1, 2023 03:32

ijsong force-pushed the grpc_server_initial_flow_control_window_size branch from 3d0cdbf to c2e0d4d Compare June 1, 2023 06:08

ijsong force-pushed the pipelined_append_handler branch from 0579220 to 918f845 Compare June 1, 2023 06:08

ijsong force-pushed the grpc_server_initial_flow_control_window_size branch from c2e0d4d to dd3061c Compare June 1, 2023 23:47

ijsong force-pushed the pipelined_append_handler branch from 918f845 to 5bc3f8b Compare June 1, 2023 23:47

ijsong mentioned this pull request Jun 2, 2023

test(benchmark): support log stream appender #462

Merged

hungryjang reviewed Jun 2, 2023

View reviewed changes

internal/storagenode/log_server.go Show resolved Hide resolved

ijsong mentioned this pull request Jun 4, 2023

ci(lint): fix depguard configuration #463

Merged

ijsong force-pushed the grpc_server_initial_flow_control_window_size branch from dd3061c to 8b623b0 Compare June 4, 2023 07:37

ijsong force-pushed the pipelined_append_handler branch from 5bc3f8b to 20e31e7 Compare June 4, 2023 07:37

Base automatically changed from grpc_server_initial_flow_control_window_size to main June 4, 2023 08:01

ijsong requested a review from hungryjang June 4, 2023 08:02

ijsong force-pushed the pipelined_append_handler branch from 20e31e7 to 4ff8d8c Compare June 4, 2023 08:05

dependabot bot and others added 9 commits June 5, 2023 12:59

Merge pull request #465 from kakao/dependabot/github_actions/golangci…

3a9688b

…/golangci-lint-action-3.5.0 build(deps): bump golangci/golangci-lint-action from 3.4.0 to 3.5.0

Merge branch 'main' into dependabot/go_modules/github.com/stretchr/te…

0d4f3db

…stify-1.8.4

Merge pull request #466 from kakao/dependabot/go_modules/github.com/s…

38046ef

…tretchr/testify-1.8.4 build(deps): bump github.com/stretchr/testify from 1.8.3 to 1.8.4

Merge pull request #467 from kakao/dependabot/go_modules/golang.org/x…

958a9a5

…/tools-0.9.3 build(deps): bump golang.org/x/tools from 0.9.1 to 0.9.3

Merge pull request #468 from kakao/dependabot/go_modules/github.com/u…

7814cd9

…rfave/cli/v2-2.25.5 build(deps): bump github.com/urfave/cli/v2 from 2.25.3 to 2.25.5

hungryjang approved these changes Jun 7, 2023

View reviewed changes

ijsong merged commit 0323629 into main Jun 7, 2023

ijsong deleted the pipelined_append_handler branch June 7, 2023 16:45

ijsong mentioned this pull request Jun 7, 2023

chore(main): release 0.14.0 #464

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(storagenode): add pipelined Append RPC handler#457

feat(storagenode): add pipelined Append RPC handler#457
ijsong merged 10 commits intomainfrom
pipelined_append_handler

ijsong commented May 23, 2023 •

edited

Loading

Uh oh!

ijsong commented May 23, 2023 •

edited

Loading

Uh oh!

codecov-commenter commented May 24, 2023 •

edited

Loading

Uh oh!

Uh oh!

ijsong commented Jun 7, 2023

Uh oh!

ijsong commented Jun 7, 2023

Uh oh!

ijsong commented Jun 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ijsong commented May 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Which issue(s) this PR resolves

Uh oh!

ijsong commented May 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented May 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

ijsong commented Jun 7, 2023

Uh oh!

ijsong commented Jun 7, 2023

Uh oh!

ijsong commented Jun 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ijsong commented May 23, 2023 •

edited

Loading

ijsong commented May 23, 2023 •

edited

Loading

codecov-commenter commented May 24, 2023 •

edited

Loading