Skip to content

Conditionally use locks for the diagnostic.log file#72

Merged
jvillafanez merged 1 commit intomasterfrom
lock_logging_file
May 20, 2021
Merged

Conditionally use locks for the diagnostic.log file#72
jvillafanez merged 1 commit intomasterfrom
lock_logging_file

Conversation

@jvillafanez
Copy link
Member

Fixes #71

You'll likely need multiple servers connected via NFS and a heavy load to trigger the problem. Note that the file is written at the end of the request.

Taking into account that the problem has been reproduced with a custom isolated script, and that locking the file before writing has solved the problem, this is a preemptive action. Reproducing the problem with ownCloud hasn't been possible yet outside of what has been seen in the issue.
In addition, due to the performance hit of having to lock the file (and maybe waiting for the file to be unlocked), this locking mechanism is disabled by default because it might not be needed in simple installations.

Note that the checks has been performed with NFS v4.1. This solution hasn't been tested with NFS v3, and it might not work there.

Test script, for reference below. Note that the "testfile.log" is written inside a NFS directory

<?php
$v = $argv[1] ?? rand();
$cc = '{"type":"QUERY","reqId":"NVDLCkoWjtVvSaa9J0Kz","diagnostics":{"sqlStatement":"SELECT `fileid`, `storage`, `path`, `parent`, `name`, `mimetype`, `mimepart`, `size`, `mtime`,    `storage_mtime`, `encrypted`, `etag`, `permissions`, `checksum` FROM `oc_filecache` WHERE `storage` = ? AND `path_hash` = ?","sqlParams":"array (   0 => 3,   1 => \"6f90b186c27cea195ea22635db5b5b35\", )","sqlQueryDurationmsec":0.9229183197021484,"sqlTimestamp":1618306963.760972}}';
$json = [];
$json['arg1'] = $v;
$json00 = json_decode($cc, true);

for ($i=0; $i<50; $i++) {
  $json['arg2'] = $i;
  $json['t0'] = $json00;
  $f = fopen('testfile.log', 'a');
  flock($f,  LOCK_EX);
  fwrite($f, json_encode($json) . "\n");
  flock($f,  LOCK_UN);
  fclose($f);
}

@CLAassistant
Copy link

CLAassistant commented Apr 14, 2021

CLA assistant check
All committers have signed the CLA.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@jvillafanez jvillafanez merged commit 14ffed5 into master May 20, 2021
@delete-merged-branch delete-merged-branch bot deleted the lock_logging_file branch May 20, 2021 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Diagnostic file writing isn't atomic nor serialized

3 participants