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

Backwards seek window does not affect the read window #999

Merged
merged 3 commits into from
Sep 6, 2024

Conversation

vladem
Copy link
Contributor

@vladem vladem commented Sep 5, 2024

Description of change

This change fixes the situation when the read_window_end_offset set on the client would be more than the configured read window above the last read offset by client application. This was happening because we were accounting some bytes from the client twice -- first time it was read from the receiver (client) and the second time when it was read from current_part (which contains bytes from backwards seek window, when backwards seek occurs).

Relevant issues: #987

Does this change impact existing behavior?

This is a part of memory control change, which affects existing behaviour but not in a breaking way.

Does this change need a changelog entry in any of the crates?

This is a part of memory control change, which should have a changelog entry.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

@vladem vladem temporarily deployed to PR integration tests September 5, 2024 12:05 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests September 5, 2024 12:05 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests September 5, 2024 12:05 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests September 5, 2024 12:05 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests September 5, 2024 12:05 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests September 5, 2024 12:05 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests September 5, 2024 12:05 — with GitHub Actions Inactive
@vladem vladem force-pushed the backwards-window-mem-control branch from ea0a89b to 652a559 Compare September 5, 2024 14:35
@vladem vladem temporarily deployed to PR integration tests September 5, 2024 14:35 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests September 5, 2024 14:35 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests September 5, 2024 14:35 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests September 5, 2024 14:35 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests September 5, 2024 14:35 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests September 5, 2024 14:35 — with GitHub Actions Inactive
@vladem vladem temporarily deployed to PR integration tests September 5, 2024 14:35 — with GitHub Actions Inactive
@vladem vladem added the performance PRs to run benchmarks on label Sep 5, 2024
@vladem vladem marked this pull request as ready for review September 5, 2024 17:01
@vladem vladem force-pushed the backwards-window-mem-control branch from 652a559 to 53f0e67 Compare September 6, 2024 12:32
@vladem vladem temporarily deployed to PR integration tests September 6, 2024 12:32 — with GitHub Actions Inactive
@vladem vladem added this pull request to the merge queue Sep 6, 2024
Merged via the queue into awslabs:main with commit 1db78f3 Sep 6, 2024
26 checks passed
@vladem vladem deleted the backwards-window-mem-control branch September 6, 2024 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance PRs to run benchmarks on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants