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

[12] fix moving folders out of a cache jail #5655

Merged
merged 6 commits into from Jul 13, 2017

Conversation

Projects
None yet
7 participants
@icewind1991
Member

icewind1991 commented Jul 8, 2017

Backport of #5424

@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Jul 8, 2017

@icewind1991, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rullzer, @nickvergessen and @LukasReschke to be potential reviewers.

mention-bot commented Jul 8, 2017

@icewind1991, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rullzer, @nickvergessen and @LukasReschke to be potential reviewers.

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Jul 8, 2017

Codecov Report

Merging #5655 into stable12 will increase coverage by 0.02%.
The diff coverage is 87.5%.

@@              Coverage Diff               @@
##             stable12    #5655      +/-   ##
==============================================
+ Coverage       54.09%   54.12%   +0.02%     
- Complexity      22408    22422      +14     
==============================================
  Files            1379     1380       +1     
  Lines           85706    85767      +61     
  Branches         1329     1329              
==============================================
+ Hits            46365    46422      +57     
- Misses          39341    39345       +4
Impacted Files Coverage Δ Complexity Δ
version.php 0% <0%> (ø) 0 <0> (ø) ⬇️
lib/private/Repair.php 27.84% <0%> (-0.36%) 19 <0> (ø)
lib/private/Files/Cache/Wrapper/CacheJail.php 88.09% <100%> (+0.29%) 38 <1> (+1) ⬆️
lib/private/Repair/NC13/RepairInvalidPaths.php 93.1% <93.1%> (ø) 13 <13> (?)
lib/private/Files/Cache/Propagator.php 94.93% <0%> (-1.27%) 16% <0%> (ø)
lib/private/Security/CertificateManager.php 90.81% <0%> (-1.03%) 39% <0%> (ø)
lib/private/Server.php 93.45% <0%> (+0.14%) 120% <0%> (ø) ⬇️
apps/comments/lib/EventHandler.php 87.5% <0%> (+8.33%) 7% <0%> (ø) ⬇️

codecov bot commented Jul 8, 2017

Codecov Report

Merging #5655 into stable12 will increase coverage by 0.02%.
The diff coverage is 87.5%.

@@              Coverage Diff               @@
##             stable12    #5655      +/-   ##
==============================================
+ Coverage       54.09%   54.12%   +0.02%     
- Complexity      22408    22422      +14     
==============================================
  Files            1379     1380       +1     
  Lines           85706    85767      +61     
  Branches         1329     1329              
==============================================
+ Hits            46365    46422      +57     
- Misses          39341    39345       +4
Impacted Files Coverage Δ Complexity Δ
version.php 0% <0%> (ø) 0 <0> (ø) ⬇️
lib/private/Repair.php 27.84% <0%> (-0.36%) 19 <0> (ø)
lib/private/Files/Cache/Wrapper/CacheJail.php 88.09% <100%> (+0.29%) 38 <1> (+1) ⬆️
lib/private/Repair/NC13/RepairInvalidPaths.php 93.1% <93.1%> (ø) 13 <13> (?)
lib/private/Files/Cache/Propagator.php 94.93% <0%> (-1.27%) 16% <0%> (ø)
lib/private/Security/CertificateManager.php 90.81% <0%> (-1.03%) 39% <0%> (ø)
lib/private/Server.php 93.45% <0%> (+0.14%) 120% <0%> (ø) ⬇️
apps/comments/lib/EventHandler.php 87.5% <0%> (+8.33%) 7% <0%> (ø) ⬇️
@MorrisJobke

Works 👍

@MorrisJobke MorrisJobke requested a review from rullzer Jul 10, 2017

@LukasReschke

This comment has been minimized.

Show comment
Hide comment
@LukasReschke

LukasReschke Jul 11, 2017

Member

Do we have some idea about the performance here if you have like 20 million files in the file cache? What is the execution time here?

If this would take ages we may need to have a repair step if someone enters a folder or so.

Member

LukasReschke commented Jul 11, 2017

Do we have some idea about the performance here if you have like 20 million files in the file cache? What is the execution time here?

If this would take ages we may need to have a repair step if someone enters a folder or so.

@rullzer

This comment has been minimized.

Show comment
Hide comment
@rullzer

rullzer Jul 12, 2017

Member

@LukasReschke well it should only happen on files that got corrupted. And if you have 20k corrupted files then we still should fix them all...

From my POV a PR that fixes it properly on update is way better than all kinds of dark magic when entering a folder... because that will basically require extra queries on each access...

Member

rullzer commented Jul 12, 2017

@LukasReschke well it should only happen on files that got corrupted. And if you have 20k corrupted files then we still should fix them all...

From my POV a PR that fixes it properly on update is way better than all kinds of dark magic when entering a folder... because that will basically require extra queries on each access...

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Jul 12, 2017

Codecov Report

Merging #5655 into stable12 will increase coverage by 0.03%.
The diff coverage is 89.53%.

@@              Coverage Diff               @@
##             stable12    #5655      +/-   ##
==============================================
+ Coverage       54.09%   54.13%   +0.03%     
- Complexity      22413    22433      +20     
==============================================
  Files            1379     1380       +1     
  Lines           85719    85802      +83     
  Branches         1329     1329              
==============================================
+ Hits            46368    46445      +77     
- Misses          39351    39357       +6
Impacted Files Coverage Δ Complexity Δ
version.php 0% <0%> (ø) 0 <0> (ø) ⬇️
lib/private/Repair.php 27.84% <0%> (-0.36%) 19 <0> (ø)
lib/private/Files/Cache/Wrapper/CacheJail.php 88.09% <100%> (+0.29%) 38 <1> (+1) ⬆️
lib/private/Repair/NC13/RepairInvalidPaths.php 93.75% <93.75%> (ø) 19 <19> (?)

codecov-io commented Jul 12, 2017

Codecov Report

Merging #5655 into stable12 will increase coverage by 0.03%.
The diff coverage is 89.53%.

@@              Coverage Diff               @@
##             stable12    #5655      +/-   ##
==============================================
+ Coverage       54.09%   54.13%   +0.03%     
- Complexity      22413    22433      +20     
==============================================
  Files            1379     1380       +1     
  Lines           85719    85802      +83     
  Branches         1329     1329              
==============================================
+ Hits            46368    46445      +77     
- Misses          39351    39357       +6
Impacted Files Coverage Δ Complexity Δ
version.php 0% <0%> (ø) 0 <0> (ø) ⬇️
lib/private/Repair.php 27.84% <0%> (-0.36%) 19 <0> (ø)
lib/private/Files/Cache/Wrapper/CacheJail.php 88.09% <100%> (+0.29%) 38 <1> (+1) ⬆️
lib/private/Repair/NC13/RepairInvalidPaths.php 93.75% <93.75%> (ø) 19 <19> (?)
@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Jul 13, 2017

Member

@icewind1991 Please rebase to fix the autoloader failure.

Member

MorrisJobke commented Jul 13, 2017

@icewind1991 Please rebase to fix the autoloader failure.

icewind1991 and others added some commits Jun 15, 2017

fix moving folders out of a cache jail
Signed-off-by: Robin Appelman <robin@icewind.nl>
Add repair step for invalid paths
Signed-off-by: Robin Appelman <robin@icewind.nl>
Run repair step only once
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
adjust to moved repair step
Signed-off-by: Robin Appelman <robin@icewind.nl>
use a generator instead of fetching all rows at once
Signed-off-by: Robin Appelman <robin@icewind.nl>
chunk getting invalid paths and reuse queries
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991

This comment has been minimized.

Show comment
Hide comment
@icewind1991

icewind1991 Jul 13, 2017

Member

@icewind1991 Please rebase to fix the autoloader failure.

Done

Member

icewind1991 commented Jul 13, 2017

@icewind1991 Please rebase to fix the autoloader failure.

Done

@rullzer

Looks good to me. Lets do this!

@MorrisJobke MorrisJobke merged commit 9097204 into stable12 Jul 13, 2017

1 check failed

continuous-integration/drone/pr the build failed
Details

@MorrisJobke MorrisJobke deleted the moveFromCache-from-shared-12 branch Jul 13, 2017

@MorrisJobke MorrisJobke referenced this pull request Jul 13, 2017

Merged

Fixed repair step #5715

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Jul 13, 2017

Member

Ported the added fixes to master: #5715

Member

MorrisJobke commented Jul 13, 2017

Ported the added fixes to master: #5715

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment