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

file.go: Added SaveToFileConcurrentWithTimeout #39

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

Conversation

ehnuje
Copy link

@ehnuje ehnuje commented Sep 21, 2020

  • Related to Support timeout for SaveToFile/SaveToFileConcurrent #38
  • When calling SaveToFileConcurrent, if the cached data is too huge, it can take quite a lot of time.
  • In our case, about 50GB of cached data takes about 7 minutes when given with `runtime.NumCPU()/2'
  • 7 minutes is too long to wait and as we can get benefit from only the partial part of the cached data, it would be nice if we can give timeout to the SaveToFileConcurrent and store as much as possible for the given time.
  • SaveToFileConcurrentWithTimeout is added to support that feature.

// The saved data may be loaded with LoadFromFile*.
//
// See also SaveToFileConcurrent.
func (c *Cache) SaveToFileConcurrentWithTimeout(filePath string, concurrency int, timeout time.Duration) error {

Choose a reason for hiding this comment

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

I suppose that passing io.Writer instead of filepath (and opening file inside) will be much more flexible and easier to test. WDYT?

Copy link
Author

Choose a reason for hiding this comment

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

@cristaloleg Thanks for the comment but I'm sorry that I don't quite understand your point "passing io.Writer and opening the file inside" 😭 Could you please elaborate? In this change, I wanted to make the function signature look like existing SaveToFileConcurrent.

Choose a reason for hiding this comment

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

I was thinking about slightly another API:

func (c *Cache) SaveConcurrentWithTimeout(w io.Writer, concurrency int, timeout time.Duration) error {

Anyway, I'm the author of fastcache, someone else might have another view on this.

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

2 participants