Skip to content

Better handling of DB timeouts on long downloads - #36978

Merged
micbar merged 2 commits into
masterfrom
long_downloads
Mar 2, 2020
Merged

Better handling of DB timeouts on long downloads#36978
micbar merged 2 commits into
masterfrom
long_downloads

Conversation

@jvillafanez

Copy link
Copy Markdown
Member

Description

Try to avoid hitting the DB after a long download, which could cause a failure if the DB connection timed out

Related Issue

https://github.com/owncloud/enterprise/issues/3538

Motivation and Context

How Has This Been Tested?

Checked after setting a 10 sec DB timeout and then downloading a big file through a slow network.
The first time, we still need to hit the DB to update the checksum (currently we're logging a message and keep working), but once the checksum is there, there won't be any error.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE
@update-docs

update-docs Bot commented Feb 19, 2020

Copy link
Copy Markdown

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@jvillafanez
jvillafanez requested a review from IljaN February 19, 2020 08:57
@codecov

codecov Bot commented Feb 19, 2020

Copy link
Copy Markdown

Codecov Report

Merging #36978 into master will decrease coverage by 0.46%.
The diff coverage is 62.5%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #36978      +/-   ##
============================================
- Coverage     64.75%   64.28%   -0.47%     
- Complexity    19138    19139       +1     
============================================
  Files          1270     1270              
  Lines         74912    74879      -33     
  Branches       1329     1329              
============================================
- Hits          48507    48134     -373     
- Misses        26014    26354     +340     
  Partials        391      391
Flag Coverage Δ Complexity Δ
#javascript 54.17% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 65.4% <62.5%> (-0.53%) 19139 <0> (+1)
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/View.php 84.54% <0%> (-0.3%) 389 <0> (ø)
lib/private/Files/Storage/Wrapper/Checksum.php 97.05% <71.42%> (-2.95%) 26 <0> (+1)
lib/private/DB/MySQLMigrator.php 0% <0%> (-100%) 5% <0%> (ø)
lib/private/DB/PostgreSqlMigrator.php 0% <0%> (-100%) 4% <0%> (ø)
...ilder/ExpressionBuilder/PgSqlExpressionBuilder.php 0% <0%> (-88.89%) 3% <0%> (ø)
lib/private/Repair/Collation.php 0% <0%> (-87.5%) 12% <0%> (ø)
...ivate/Files/ObjectStore/HomeObjectStoreStorage.php 0% <0%> (-86.67%) 8% <0%> (ø)
lib/private/DB/AdapterPgSql.php 0% <0%> (-85.72%) 2% <0%> (ø)
lib/private/Repair/InnoDB.php 0% <0%> (-78.95%) 6% <0%> (ø)
...ilder/ExpressionBuilder/MySqlExpressionBuilder.php 0% <0%> (-62.5%) 3% <0%> (ø)
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5176469...a7eeab4. Read the comment docs.

@jvillafanez

Copy link
Copy Markdown
Member Author

I can't increase the coverage. The cache can't be mocked, so we can't force the DriverException to trigger

@IljaN IljaN left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 💃

@micbar
micbar merged commit a165f2b into master Mar 2, 2020
@delete-merged-branch
delete-merged-branch Bot deleted the long_downloads branch March 2, 2020 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

4 participants