Skip to content

issue-1751: Resolve concerns in TWriteBackCache #3690

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

issue-1751: Resolve concerns in TWriteBackCache #3690

wants to merge 6 commits into from

Conversation

e673
Copy link
Collaborator

@e673 e673 commented Jun 17, 2025 •
edited
Loading

-- This PR is to be split into several PRs --

TRingBuffer:

TWriteBackCache:

  • Get rid of double copy (multiple places)
  • Store request buffer only once
  • Execute simultaneously only one Flush operation per handle
  • Copy request buffer to cache outside lock

Unit test:

  • Fix data race

ToDo:

  • Unit tests for TWriteBackCache::TImpl::InvertDataParts
  • Asynchronous execution of pending actions
  • Use retry policy

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 17, 2025 •
edited
Loading

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 7fd01c7.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
8651 8644 0 6 1 0

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 7fd01c7.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
53 47 0 6 0 0

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 7fd01c7.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
53 47 0 6 0 0

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 17, 2025 •
edited
Loading

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 80d69fb.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
8651 8645 0 5 1 0

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 80d69fb.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
51 46 0 5 0 0

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 80d69fb.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
51 46 0 5 0 0

issue-1751: get rid of double copy in TWriteBackCache; allow TRingBu…
6f7e45a 
…ffer to be >4GB and use padding; handle errors during Flush; allow only single Flush per handle; resolve concerns
@e673 e673 force-pushed the users/nasonov/issue-1751/2 branch from 80d69fb to 6f7e45a Compare June 18, 2025 01:13
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 18, 2025 •
edited
Loading

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 6f7e45a.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
8648 8643 0 4 1 0

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 6f7e45a.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
77 77 0 0 0 0

@e673 e673 requested a review from SvartMetal June 19, 2025 13:12
@e673 e673 marked this pull request as ready for review June 19, 2025 13:12
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 19, 2025 •
edited
Loading

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 651949e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
8651 8648 0 0 1 2

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 651949e.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
112 112 0 0 0 0

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 21, 2025 •
edited
Loading

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit b756043.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
8651 8649 0 1 1 0

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit b756043.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
2 2 0 0 0 0

@e673
Copy link
Collaborator Author

e673 commented Jun 21, 2025

This PR will be split into several PRs

@e673 e673 mentioned this pull request Jun 23, 2025
Merged
@e673 e673 removed the request for review from SvartMetal June 23, 2025 06:58
e673 added a commit that referenced this pull request Jun 27, 2025
@e673
issue-1751: TFileRingBuffer supports > 4GB capacity; do not zero the …
c4798a4 
…entire slackspace; fully utilize capacity (#3734)

Support > 4GB capacity by TFileRingBuffer.

Minor improvements:

There is no need to zero the entire slack space, only header.
Fully utilize free space if there is a single contiguous occupied region.
Related to
#1751

Extracted from
#3690
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@e673