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

Add config value for TTL of files in cache folder before removed by garbage collection #23486

Closed
wants to merge 2 commits into from
Closed

Add config value for TTL of files in cache folder before removed by garbage collection #23486

wants to merge 2 commits into from

Conversation

fkammer
Copy link
Contributor

@fkammer fkammer commented Mar 22, 2016

Closes #23455

Excerpt from issue:

Steps to reproduce

  1. Start uploading a very large file (e.g. 10 GB) via owncloud client
  2. Use uploading computer as usual (let it go to sleep, shut it down over night, don't have an internet connection all the time)

Expected behaviour

  • ocClient starts to upload the large file in 5MB chunks.
  • It pauses once in a while (no connection, computer goes to sleep or is shut down).
  • It resumes uploading after the connection is established again
  • Upload finishes after all chunks are uploaded

Actual behaviour

  • ocClient starts to upload the large file in 5MB chunks.
  • It pauses once in a while (no connection, computer goes to sleep or is shut down).
  • It resumes uploading after the connection is established again
  • Chunks older than one day are removed by the Garbage Collection
  • ocClient detects that a good amount of chunks is missing (as they were deleted by GC)
  • Starts to upload these chunks again
  • During that, other now outdated chunks are deleted

Problem: The upload takes forever as the client is not able to upload all chunks in 24 hours.

@DeepDiver1975 DeepDiver1975 added this to the 9.1-current milestone Mar 22, 2016
@DeepDiver1975
Copy link
Member

I'm wondering if we better set this in the chunking code instead of adjusting the default which is used in different places as well.

https://github.com/owncloud/core/blob/master/lib/private/filechunking.php#L70

@fkammer
Copy link
Contributor Author

fkammer commented Mar 23, 2016

@DeepDiver1975 Do you mean changing

public function store($index, $data) {
    $cache = $this->getCache();
    $name = $this->getPrefix().$index;
    $cache->set($name, $data);
    return $cache->size($name);
}

to

public function store($index, $data) {
    $cache = $this->getCache();
    $name = $this->getPrefix().$index;
    $ttl = \OC::$server->getConfig()->getSystemValue('cache_folder_gc_ttl', 86400);
    $cache->set($name, $data, $ttl);
    return $cache->size($name);
}

in https://github.com/owncloud/core/blob/master/lib/private/filechunking.php#L67-L73

@fkammer
Copy link
Contributor Author

fkammer commented Mar 23, 2016

I was wondering if it would be more elegant to update the mtime of all corresponding chunks if a new one is added. That should fix the problem for all installations without the need to configure the new variable.

Nevertheless could we keep that variable if someone experiences problems while uploading a large file over multiple days and can't ensure that at least one chunk is uploaded every 24 hours.

@PVince81
Copy link
Contributor

I was wondering if it would be more elegant to update the mtime of all corresponding chunks if a new one is added.

That would be expensive, you'd need to touch the mtime of all existing chunks (which could be a lot) while leaving the current one untouched.

@PVince81
Copy link
Contributor

@fkammer also, yes I think #23486 (comment) is correct. Move the config value to the "FileChunking" class.

@PVince81
Copy link
Contributor

Looks like the TTL value passed in 'set()` is also likely to cause other trouble, see #24653

Maybe we should get rid of it or make it instance-global instead of file-specific.

@PVince81
Copy link
Contributor

Moved to #24812 that implements the correct approach.

@PVince81 PVince81 closed this May 24, 2016
@lock
Copy link

lock bot commented Aug 5, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make chunk cache ttl configurable
4 participants