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.

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

Minor improvements in file cache #333

Merged
merged 7 commits into from
Oct 11, 2024
Merged

Minor improvements in file cache #333

merged 7 commits into from
Oct 11, 2024

Conversation

foodprocessor
Copy link
Contributor

@foodprocessor foodprocessor commented Sep 28, 2024 •
edited
Loading

Describe your changes in brief

In file cache test suite:

  • Re-enable tests that were flaky due to race conditions that are fixed now
  • Shorten a couple test sleep times
  • Remove unnecessary OpenFile when timeout is non-zero
  • Remove redundant call to setupTestHelper with config identical to test default config

In lru_policy:

  • Remove unused const
  • Simplify cacheTimeoutMonitor code
    • (calling time.Tick with a zero interval already returns nil, so no need to check if the interval is zero)
  • Add comments to updateMarker to explain linked list code

What type of Pull Request is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Checklist

  • Tested locally
  • Added new dependencies
  • Updated documentation
  • Added tests

Related Issues

  • Related Issue #
  • Closes #

@foodprocessor foodprocessor changed the title Re-enable previously flaky tests Minor improvements in file cache Sep 28, 2024
Dabnsky
Dabnsky approved these changes Oct 4, 2024
View reviewed changes
Copy link
Contributor

@Dabnsky Dabnsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks pretty good to me. Great job

Ferelith-maker
Ferelith-maker approved these changes Oct 10, 2024
View reviewed changes
Copy link
Contributor

@Ferelith-maker Ferelith-maker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your comments make sense. Good job

@foodprocessor foodprocessor merged commit ed9d93c into main Oct 11, 2024
14 of 17 checks passed
@foodprocessor foodprocessor deleted the minor-improvements-to-filecache branch October 11, 2024 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ferelith-maker Ferelith-maker Ferelith-maker approved these changes

@Dabnsky Dabnsky Dabnsky approved these changes

@jfantinhardesty jfantinhardesty Awaiting requested review from jfantinhardesty jfantinhardesty is a code owner

@brayan-trejo brayan-trejo Awaiting requested review from brayan-trejo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@foodprocessor @Dabnsky @Ferelith-maker