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

Client shouldn't chunk in case no chunking capabilities were announced. #7862

Closed
TheOneRing opened this issue May 12, 2020 · 5 comments
Closed
Assignees
Labels
p2-high Escalation, on top of current planning, release blocker
Milestone

Comments

@TheOneRing
Copy link
Member

TheOneRing commented May 12, 2020

#7849

See related owncloud/ocis#195

Whenever the capability "bigfilechunking" is set to false, the desktop client must not attempt to use the chunking v1 algorithm (this one

headers[QByteArrayLiteral("OC-Chunked")] = QByteArrayLiteral("1");
)

the capability attribute is in:

"ocs": {
    "data": {
      ...
      "capabilities": {
        ...
        "files": {
          ...
          "bigfilechunking": false,
        },

I guess the check was never implemented in the desktop client as chunking v1 was always supported by OC from start. So now dealing with OCIS with no or broken support makes this check necessary.


Similar when OWNCLOUD_CHUNK_SIZE=0 was set

Also in case:
"bigfilechunking": false,

Related:
owncloud/ocis#203
owncloud/ocis#272

@michaelstingl michaelstingl added this to the 2.7.0 milestone May 12, 2020
@michaelstingl michaelstingl added the p2-high Escalation, on top of current planning, release blocker label May 12, 2020
@jnweiger jnweiger mentioned this issue Oct 13, 2020
63 tasks
@jnweiger
Copy link
Contributor

jnweiger commented Oct 23, 2020

Chunking settings are in multiple places in the capabulities report:

curl -s -u admin:admin 'http://pigpen.jw-qa.owncloud.works/ocs/v1.php/cloud/capabilities?format=json'  | jq '.ocs.data.capabilities.dav,.ocs.data.capabilities.files'

.ocs.data.capabilities.dav: { "chunking": "1.0", ...
.ocs.data.capabilities.files: { "bigfilechunking": true, ...

These capabilities are normally hardcoded per server. There is no GUI or config file to tune them.
For testing purposes, edited /var/www/owncloud/apps/{dav,files}/lib/Capabilities.php -- Edit at own risk :-)

@jnweiger jnweiger reopened this Oct 28, 2020
@jnweiger
Copy link
Contributor

jnweiger commented Oct 28, 2020

Tested with
testpilotcloud 2.7.0daily20201019 (build 2311) on Linux and 2.7.0rc1 on windows
against a 10.5.0 server:

curl -s -u admin:admin 'http://pigpen.jw-qa.owncloud.works/status.php'
{"installed":true,"maintenance":false,"needsDbUpgrade":false,"version":"10.5.0.10","versionstring":"10.5.0","edition":"Community","productname":"ownCloud"}

We disable all chunking support on the server side by editing apps/dav/lib/Capabilities.php and apps/files/lib/Capabilities.php so that the server does not announce any chunking capabilities:

curl -s -u admin:admin 'http://pigpen.jw-qa.owncloud.works/ocs/v1.php/cloud/capabilities?format=json'  | jq . | grep chunk
          "bigfilechunking": false,
          "chunking": "",

Steps to reproduce

  • make sure, no CHUNKING environment variables are set: env | grep CHUNK reports nothing.
  • connect client to server, with logging enabled
  • wait for initial sync
  • move a 42MB file into the desktop sync folder
  • wait until upload finishes
  • check the logfiles
zgrep -i nextchunk 20201028_1132_owncloud.log.0.gz 
10-28 11:32:18:534 [ debug sync.propagator.upload.v1 ]	[ OCC::PropagateUploadFileV1::startNextChunk ]:	5 false 0 10000000
10-28 11:32:18:535 [ debug sync.propagator.upload.v1 ]	[ OCC::PropagateUploadFileV1::startNextChunk ]:	5 false 10000000 10000000
10-28 11:32:18:535 [ debug sync.propagator.upload.v1 ]	[ OCC::PropagateUploadFileV1::startNextChunk ]:	5 false 20000000 10000000
10-28 11:33:34:715 [ debug sync.propagator.upload.v1 ]	[ OCC::PropagateUploadFileV1::startNextChunk ]:	5 false 30000000 10000000
10-28 11:34:20:953 [ debug sync.propagator.upload.v1 ]	[ OCC::PropagateUploadFileV1::startNextChunk ]:	5 true 40000000 3994768

The client did chunking.

Expected behaviour: no chunking.

@TheOneRing TheOneRing self-assigned this Oct 28, 2020
@TheOneRing TheOneRing pinned this issue Oct 28, 2020
@jnweiger
Copy link
Contributor

jnweiger commented Oct 28, 2020

Tried more combinations too:

  • removed both 'chunking' and 'bigfilechunking' completely from the capabilities
  • removed chunking, but preserve 'bigfilechunking=true' as old servers did
    Always same results. 10MB chunking kicks in.

20201028_1148_owncloud.log.0.gz
20201028_1147_owncloud.log.0.gz
20201028_1146_owncloud.log.2.gz
20201028_1146_owncloud.log.1.gz
20201028_1146_owncloud.log.0.gz
20201028_1144_owncloud.log.0.gz
20201028_1135_owncloud.log.0.gz
20201028_1134_owncloud.log.0.gz
20201028_1132_owncloud.log.0.gz

@jnweiger
Copy link
Contributor

jnweiger commented Oct 28, 2020

We need to restart the client, when changing the settings.
Changing server capabilities while the client is connected is unrealistic, given that these values are normally hardcoded per server.

Retesting the 42MB file upload with restarting the client before changing the server capabilities:

usecase bigfilechunking chunking desktop client 2.7.0 rc1
oC 7.0 .. 9.1 true undefined "Chunking V1": exactly 10MB
true "" "Chunking V1": exactly 10MB
oC 10.0.2 true "1.0" "Chunking NG": upload of 10000000, 22019345, 11975423
false undefined one big chunk: startNextChunk ]: 1 true 0 43994768
OC-Chunk-Size: 10000000, OC-Total-Length: 43994768,
ocis-beta5 false "" one big chunk: startNextChunk ]: 1 true 0 43994768
OC-Chunk-Size: 10000000, OC-Total-Length: 43994768,
false "1.0" one big chunk: startNextChunk ]: 1 true 0 43994768
OC-Chunk-Size: 10000000, OC-Total-Length: 43994768,

Usecases were not really tested with these server versions. Recorded here as seen in owncloud/product#39

@TheOneRing TheOneRing unpinned this issue Oct 28, 2020
@jnweiger
Copy link
Contributor

Takeaway:

  • We ignore the reported OC-Chunk-Size in the logs -- the actual upload is in one big chunk and is considered "no chunking".
  • The capability `chunking="1.0" does not enable "Chunking V1" -- it switches to "NG" instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-high Escalation, on top of current planning, release blocker
Projects
None yet
Development

No branches or pull requests

3 participants