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

refactor(logstream): consolidate append wait groups into a single object #995

Open
wants to merge 1 commit into
base: deprecate_error_field_in_appendresult
Choose a base branch
from

Conversation

ijsong
Copy link
Member

@ijsong ijsong commented Feb 20, 2025

What this PR does

This PR reduces append wait groups to a single object for each append batch. The
append wait groups are part of the following structs:

  • appendContext
  • commitWaitTask
  • sequenceTask

Previously, multiple append wait groups existed because each log entry in a
batch had its own wait group. With the changes for issue #843, a single append
wait group now corresponds to an entire batch. This PR consolidates the list of
append wait groups into a single append wait group.

TODO: We are still on the road to resolving #843. The rest of the work mostly
involves cleaning up the code base.

@ijsong ijsong requested a review from hungryjang as a code owner February 20, 2025 10:46
@ijsong ijsong force-pushed the consolidate_append_wait_groups_into_a_single_object branch from 44d1fb0 to 3b5d0de Compare February 20, 2025 23:26
Copy link

codecov bot commented Feb 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (deprecate_error_field_in_appendresult@1902acb). Learn more about missing BASE report.

Additional details and impacted files
@@                           Coverage Diff                            @@
##             deprecate_error_field_in_appendresult     #995   +/-   ##
========================================================================
  Coverage                                         ?   79.66%           
========================================================================
  Files                                            ?      178           
  Lines                                            ?    21475           
  Branches                                         ?        0           
========================================================================
  Hits                                             ?    17109           
  Misses                                           ?     3576           
  Partials                                         ?      790           

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

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ijsong ijsong force-pushed the deprecate_error_field_in_appendresult branch from 4511a81 to f172616 Compare February 21, 2025 09:00
@ijsong ijsong force-pushed the consolidate_append_wait_groups_into_a_single_object branch from 3b5d0de to 1ad1abf Compare February 21, 2025 09:01
@ijsong ijsong force-pushed the deprecate_error_field_in_appendresult branch from f172616 to c119d34 Compare February 21, 2025 10:16
@ijsong ijsong force-pushed the consolidate_append_wait_groups_into_a_single_object branch from 1ad1abf to 44b49d5 Compare February 21, 2025 10:16
@ijsong ijsong force-pushed the deprecate_error_field_in_appendresult branch from c119d34 to b3d4566 Compare February 21, 2025 12:59
@ijsong ijsong force-pushed the consolidate_append_wait_groups_into_a_single_object branch from 44b49d5 to 5f405f7 Compare February 21, 2025 12:59
@ijsong ijsong force-pushed the deprecate_error_field_in_appendresult branch from b3d4566 to 0515210 Compare February 21, 2025 13:20
@ijsong ijsong force-pushed the consolidate_append_wait_groups_into_a_single_object branch from 5f405f7 to ed39e3b Compare February 21, 2025 13:20
@ijsong ijsong force-pushed the deprecate_error_field_in_appendresult branch from 0515210 to b8ded9d Compare February 21, 2025 13:42
@ijsong ijsong force-pushed the consolidate_append_wait_groups_into_a_single_object branch from ed39e3b to 7b9ac3a Compare February 21, 2025 13:42
@ijsong ijsong force-pushed the deprecate_error_field_in_appendresult branch from b8ded9d to 48dd48a Compare February 21, 2025 13:55
@ijsong ijsong force-pushed the consolidate_append_wait_groups_into_a_single_object branch from 7b9ac3a to a05f7e3 Compare February 21, 2025 13:55
@ijsong ijsong force-pushed the deprecate_error_field_in_appendresult branch from 48dd48a to 58d9894 Compare March 5, 2025 11:49
@ijsong ijsong force-pushed the consolidate_append_wait_groups_into_a_single_object branch from a05f7e3 to ea29d7f Compare March 5, 2025 11:49
@ijsong ijsong force-pushed the deprecate_error_field_in_appendresult branch from 58d9894 to 3967cd2 Compare March 6, 2025 10:23
@ijsong ijsong force-pushed the consolidate_append_wait_groups_into_a_single_object branch from ea29d7f to 4695466 Compare March 6, 2025 10:23
@ijsong ijsong force-pushed the deprecate_error_field_in_appendresult branch from 3967cd2 to 1cb6821 Compare March 8, 2025 13:06
@ijsong ijsong force-pushed the consolidate_append_wait_groups_into_a_single_object branch from 4695466 to 5af75b7 Compare March 8, 2025 13:06
This PR reduces append wait groups to a single object for each append batch. The
append wait groups are part of the following structs:

- appendContext
- commitWaitTask
- sequenceTask

Previously, multiple append wait groups existed because each log entry in a
batch had its own wait group. With the changes for issue #843, a single append
wait group now corresponds to an entire batch. This PR consolidates the list of
append wait groups into a single append wait group.

TODO: We are still on the road to resolving #843. The rest of the work mostly
involves cleaning up the code base.
@ijsong ijsong force-pushed the deprecate_error_field_in_appendresult branch from 1cb6821 to 1902acb Compare March 14, 2025 14:11
@ijsong ijsong force-pushed the consolidate_append_wait_groups_into_a_single_object branch from 5af75b7 to 9838377 Compare March 14, 2025 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant