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.

Sign up for GitHub

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

Jump to bottom

add an admin check for db file locking #37766

Merged
merged 2 commits into from
Apr 18, 2023
Merged

add an admin check for db file locking #37766

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
80645a5
add an admin check for db file locking
szaimen Apr 17, 2023
e2ab9bf
address review and fix tests
szaimen Apr 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions apps/settings/lib/Controller/CheckSetupController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
use OC\DB\SchemaWrapper;
use OC\IntegrityCheck\Checker;
use OC\Lock\NoopLockingProvider;
use OC\Lock\DBLockingProvider;
use OC\MemoryInfo;
use OCA\Settings\SetupChecks\CheckUserCertificates;
use OCA\Settings\SetupChecks\LdapInvalidUuids;
Expand Down Expand Up @@ -619,6 +620,10 @@ protected function hasWorkingFileLocking(): bool {
return !($this->lockingProvider instanceof NoopLockingProvider);
}

protected function hasDBFileLocking(): bool {
return ($this->lockingProvider instanceof DBLockingProvider);
}

protected function getSuggestedOverwriteCliURL(): string {
$currentOverwriteCliUrl = $this->config->getSystemValue('overwrite.cli.url', '');
$suggestedOverwriteCliUrl = $this->request->getServerProtocol() . '://' . $this->request->getInsecureServerHost() . \OC::$WEBROOT;
Expand Down Expand Up @@ -875,6 +880,7 @@ public function check() {
'wasEmailTestSuccessful' => $this->wasEmailTestSuccessful(),
'hasFileinfoInstalled' => $this->hasFileinfoInstalled(),
'hasWorkingFileLocking' => $this->hasWorkingFileLocking(),
'hasDBFileLocking' => $this->hasDBFileLocking(),
'suggestedOverwriteCliURL' => $this->getSuggestedOverwriteCliURL(),
'cronInfo' => $this->getLastCronInfo(),
'cronErrors' => $this->getCronErrors(),
Expand Down
4 changes: 4 additions & 0 deletions apps/settings/tests/Controller/CheckSetupControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,10 @@ public function testCheck() {
->expects($this->once())
->method('hasWorkingFileLocking')
->willReturn(true);
$this->checkSetupController
->expects($this->once())
->method('hasDBileLocking')
szaimen marked this conversation as resolved.
Show resolved Hide resolved
->willReturn(true);
szaimen marked this conversation as resolved.
Show resolved Hide resolved
$this->checkSetupController
->expects($this->once())
->method('getSuggestedOverwriteCliURL')
Expand Down
8 changes: 8 additions & 0 deletions core/js/setupchecks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,14 @@
type: OC.SetupChecks.MESSAGE_TYPE_WARNING
});
}
if(data.hasDBFileLocking) {
messages.push({
msg: t('core', 'The database is used for transactional file locking. This will lead to performance issues. Use redis for transactional file locking to improve the performance. See the {linkstart}documentation ↗{linkend} for more information.')
szaimen marked this conversation as resolved.
Show resolved Hide resolved
.replace('{linkstart}', '<a target="_blank" rel="noreferrer noopener" class="external" href="' + OC.theme.docPlaceholderUrl.replace('PLACEHOLDER', 'admin-transactional-locking') + '">')
.replace('{linkend}', '</a>'),
type: OC.SetupChecks.MESSAGE_TYPE_INFO
});
}
if (data.suggestedOverwriteCliURL !== '') {
messages.push({
msg: t('core', 'Please make sure to set the "overwrite.cli.url" option in your config.php file to the URL that your users mainly use to access this Nextcloud. Suggestion: "{suggestedOverwriteCliURL}". Otherwise there might be problems with the URL generation via cron. (It is possible though that the suggested URL is not the URL that your users mainly use to access this Nextcloud. Best is to double check this in any case.)', {suggestedOverwriteCliURL: data.suggestedOverwriteCliURL}),
Expand Down
140 changes: 140 additions & 0 deletions core/js/tests/specs/setupchecksSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -289,6 +290,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -351,6 +353,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -410,6 +413,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: false,
Expand Down Expand Up @@ -468,6 +472,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -526,6 +531,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: false,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -570,6 +576,124 @@ describe('OC.SetupChecks tests', function() {
});
});

it('should return an info if transactional file locking is not set up', function(done) {
var async = OC.SetupChecks.checkSetup();

suite.server.requests[0].respond(
200,
{
'Content-Type': 'application/json'
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: false,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
isFairUseOfFreePushService: true,
serverHasInternetConnectionProblems: false,
isMemcacheConfigured: true,
forwardedForHeadersWorking: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
OpcacheSetupRecommendations: [],
isSettimelimitAvailable: true,
hasFreeTypeSupport: true,
missingIndexes: [],
missingPrimaryKeys: [],
missingColumns: [],
cronErrors: [],
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
is64bit: true,
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false,
isDefaultPhoneRegionSet: true,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true,
reverseProxyGeneratedURL: 'https://server',
temporaryDirectoryWritable: true,
})
);

async.done(function( data, s, x ){
expect(data).toEqual([{
msg: 'Transactional file locking is disabled, this might lead to issues with race conditions. Enable "filelocking.enabled" in config.php to avoid these problems. See the <a target="_blank" rel="noreferrer noopener" class="external" href="https://docs.example.org/admin-transactional-locking">documentation ↗</a> for more information.',
type: OC.SetupChecks.MESSAGE_TYPE_WARNING
}]);
done();
});
});

it('should return an info if database file locking is used', function(done) {
var async = OC.SetupChecks.checkSetup();

suite.server.requests[0].respond(
200,
{
'Content-Type': 'application/json'
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: true,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.nextcloud.com/myDocs.html',
isFairUseOfFreePushService: true,
serverHasInternetConnectionProblems: false,
isMemcacheConfigured: true,
forwardedForHeadersWorking: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
OpcacheSetupRecommendations: [],
isSettimelimitAvailable: true,
hasFreeTypeSupport: true,
missingIndexes: [],
missingPrimaryKeys: [],
missingColumns: [],
cronErrors: [],
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
isImagickEnabled: true,
areWebauthnExtensionsEnabled: true,
is64bit: true,
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false,
isDefaultPhoneRegionSet: true,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true,
reverseProxyGeneratedURL: 'https://server',
temporaryDirectoryWritable: true,
})
);

async.done(function( data, s, x ){
expect(data).toEqual([{
msg: 'The database is used for transactional file locking. This will lead to performance issues. Use redis for transactional file locking to improve the performance. See the <a target="_blank" rel="noreferrer noopener" class="external" href="https://docs.example.org/admin-transactional-locking">documentation ↗</a> for more information.',
type: OC.SetupChecks.MESSAGE_TYPE_INFO
}]);
done();
});
});

it('should return a warning if there are app directories with wrong permissions', function(done) {
var async = OC.SetupChecks.checkSetup();

Expand All @@ -584,6 +708,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -644,6 +769,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -702,6 +828,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -760,6 +887,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -838,6 +966,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -897,6 +1026,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -955,6 +1085,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -1013,6 +1144,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -1075,6 +1207,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -1134,6 +1267,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -1190,6 +1324,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -1249,6 +1384,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -1308,6 +1444,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -1366,6 +1503,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -1424,6 +1562,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down Expand Up @@ -1482,6 +1621,7 @@ describe('OC.SetupChecks tests', function() {
isReadOnlyConfig: false,
wasEmailTestSuccessful: true,
hasWorkingFileLocking: true,
hasDBFileLocking: false,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
Expand Down