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

improve index on appconfig #42903

Merged
merged 3 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
68 changes: 68 additions & 0 deletions core/Migrations/Version28000Date20231126110901.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?php

declare(strict_types=1);
/**
* @copyright 2023 Maxence Lange <maxence@artificial-owl.com>
*
* @author Maxence Lange <maxence@artificial-owl.com>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

namespace OC\Core\Migrations;

use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

/**
* Create new fields for type and lazy loading in appconfig for the new IAppConfig API.
*/
class Version28000Date20231126110901 extends SimpleMigrationStep {
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
return null;

/**
* this migration was needed during Nextcloud 28 to prep the migration to 29 and a
* new IAppConfig as its API require 'lazy' and 'type' database field.
*
* some changes in the migration process and the expected result have made its execution
* useless, therefore ignored.
*
* @see Version29000Date20240124132201
* @see Version29000Date20240124132202
*/
// /** @var ISchemaWrapper $schema */
// $schema = $schemaClosure();
//
// if (!$schema->hasTable('appconfig')) {
// return null;
// }
//
// $table = $schema->getTable('appconfig');
// if ($table->hasColumn('lazy')) {
// return null;
// }
//
// // type=2 means value is typed as MIXED
// $table->addColumn('type', Types::INTEGER, ['notnull' => true, 'default' => 2]);
// $table->addColumn('lazy', Types::BOOLEAN, ['notnull' => false, 'default' => false]);
//
// return $schema;
}
}
39 changes: 23 additions & 16 deletions core/Migrations/Version29000Date20231126110901.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,27 +39,34 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

if (!$schema->hasTable('appconfig')) {
return null;
}

$table = $schema->getTable('appconfig');
if ($table->hasColumn('lazy')) {
return null;
}

// type=2 means value is typed as MIXED
$table->addColumn('type', Types::INTEGER, ['notnull' => true, 'default' => 2]);
$table->addColumn('lazy', Types::BOOLEAN, ['notnull' => false, 'default' => false]);

if ($table->hasIndex('appconfig_config_key_index')) {
$table->dropIndex('appconfig_config_key_index');
}

$table->addIndex(['lazy'], 'ac_lazy_i');
$table->addIndex(['appid', 'lazy'], 'ac_app_lazy_i');
$table->addIndex(['appid', 'lazy', 'configkey'], 'ac_app_lazy_key_i');
/**
* This code is now useless, after a discussion about boolean on oracle;
* it has been decided to use another type for the lazy field
*
* a better migration process is available there:
*
* @see Version29000Date20240124132201 for the revert of current migration
* @see Version29000Date20240124132202 for the new migration process
*/
return null;

return $schema;
// // type=2 means value is typed as MIXED
// $table->addColumn('type', Types::INTEGER, ['notnull' => true, 'default' => 2]);
// $table->addColumn('lazy', Types::BOOLEAN, ['notnull' => false, 'default' => false]);
//
// if ($table->hasIndex('appconfig_config_key_index')) {
// $table->dropIndex('appconfig_config_key_index');
// }
//
// $table->addIndex(['lazy'], 'ac_lazy_i');
// $table->addIndex(['appid', 'lazy'], 'ac_app_lazy_i');
// $table->addIndex(['appid', 'lazy', 'configkey'], 'ac_app_lazy_key_i');
//
// return $schema;
}
}
75 changes: 75 additions & 0 deletions core/Migrations/Version29000Date20240124132201.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php

declare(strict_types=1);
/**
* @copyright 2023 Maxence Lange <maxence@artificial-owl.com>
*
* @author Maxence Lange <maxence@artificial-owl.com>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

namespace OC\Core\Migrations;

use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\DB\Types;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

/**
* Create new column for type and remove previous lazy column in appconfig (will be recreated by Version29000Date20240124132202) for the new IAppConfig API.
*/
class Version29000Date20240124132201 extends SimpleMigrationStep {
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$table = $schema->getTable('appconfig');

// we will drop 'lazy', we start to clean related indexes first
if ($table->hasIndex('ac_lazy_i')) {
$table->dropIndex('ac_lazy_i');
}

if ($table->hasIndex('ac_app_lazy_i')) {
$table->dropIndex('ac_app_lazy_i');
}

if ($table->hasIndex('ac_app_lazy_key_i')) {
$table->dropIndex('ac_app_lazy_key_i');
}

if ($table->hasColumn('lazy')) {
$table->dropColumn('lazy');
}

// create field 'type' if it does not exist yet, or fix the fact that it is missing 'unsigned'
if (!$table->hasColumn('type')) {
$table->addColumn('type', Types::INTEGER, ['notnull' => true, 'default' => 2, 'unsigned' => true]);
} else {
$table->modifyColumn('type', ['notnull' => true, 'default' => 2, 'unsigned' => true]);
}

// not needed anymore
if ($table->hasIndex('appconfig_config_key_index')) {
$table->dropIndex('appconfig_config_key_index');
}

return $schema;
}
}
53 changes: 53 additions & 0 deletions core/Migrations/Version29000Date20240124132202.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

declare(strict_types=1);
/**
* @copyright 2023 Maxence Lange <maxence@artificial-owl.com>
*
* @author Maxence Lange <maxence@artificial-owl.com>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

namespace OC\Core\Migrations;

use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\DB\Types;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

/**
* Create new column and index for lazy loading in appconfig for the new IAppConfig API.
*/
class Version29000Date20240124132202 extends SimpleMigrationStep {
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$table = $schema->getTable('appconfig');
$table->addColumn('lazy', Types::SMALLINT, ['notnull' => true, 'default' => 0, 'length' => 1, 'unsigned' => true]);

/**
* we only need an index on lazy, the group 'appid'+'configkey' already
* exists as primary ({@see Version13000Date20170718121200})
*/
$table->addIndex(['lazy'], 'ac_lazy_i');

return $schema;
}
}
3 changes: 3 additions & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1241,8 +1241,11 @@
'OC\\Core\\Migrations\\Version28000Date20230906104802' => $baseDir . '/core/Migrations/Version28000Date20230906104802.php',
'OC\\Core\\Migrations\\Version28000Date20231004103301' => $baseDir . '/core/Migrations/Version28000Date20231004103301.php',
'OC\\Core\\Migrations\\Version28000Date20231103104802' => $baseDir . '/core/Migrations/Version28000Date20231103104802.php',
'OC\\Core\\Migrations\\Version28000Date20231126110901' => $baseDir . '/core/Migrations/Version28000Date20231126110901.php',
'OC\\Core\\Migrations\\Version29000Date20231126110901' => $baseDir . '/core/Migrations/Version29000Date20231126110901.php',
'OC\\Core\\Migrations\\Version29000Date20231213104850' => $baseDir . '/core/Migrations/Version29000Date20231213104850.php',
'OC\\Core\\Migrations\\Version29000Date20240124132201' => $baseDir . '/core/Migrations/Version29000Date20240124132201.php',
'OC\\Core\\Migrations\\Version29000Date20240124132202' => $baseDir . '/core/Migrations/Version29000Date20240124132202.php',
'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php',
'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php',
'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php',
Expand Down
3 changes: 3 additions & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1274,8 +1274,11 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\Core\\Migrations\\Version28000Date20230906104802' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20230906104802.php',
'OC\\Core\\Migrations\\Version28000Date20231004103301' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20231004103301.php',
'OC\\Core\\Migrations\\Version28000Date20231103104802' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20231103104802.php',
'OC\\Core\\Migrations\\Version28000Date20231126110901' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20231126110901.php',
'OC\\Core\\Migrations\\Version29000Date20231126110901' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20231126110901.php',
'OC\\Core\\Migrations\\Version29000Date20231213104850' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20231213104850.php',
'OC\\Core\\Migrations\\Version29000Date20240124132201' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20240124132201.php',
'OC\\Core\\Migrations\\Version29000Date20240124132202' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20240124132202.php',
'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php',
'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php',
'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php',
Expand Down
43 changes: 23 additions & 20 deletions lib/private/AppConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ class AppConfig implements IAppConfig {
/**
* $migrationCompleted is only needed to manage the previous structure
* of the database during the upgrading process to nc29.
*
* only when upgrading from a version prior 28.0.2
*
* @TODO: remove this value in Nextcloud 30+
*/
private bool $migrationCompleted = true;
Expand Down Expand Up @@ -735,7 +738,7 @@ private function setTypedValue(
$insert = $this->connection->getQueryBuilder();
$insert->insert('appconfig')
->setValue('appid', $insert->createNamedParameter($app))
->setValue('lazy', $insert->createNamedParameter($lazy, IQueryBuilder::PARAM_BOOL))
->setValue('lazy', $insert->createNamedParameter(($lazy) ? 1 : 0, IQueryBuilder::PARAM_INT))
->setValue('type', $insert->createNamedParameter($type, IQueryBuilder::PARAM_INT))
->setValue('configkey', $insert->createNamedParameter($key))
->setValue('configvalue', $insert->createNamedParameter($value));
Expand Down Expand Up @@ -788,7 +791,7 @@ private function setTypedValue(
$update = $this->connection->getQueryBuilder();
$update->update('appconfig')
->set('configvalue', $update->createNamedParameter($value))
->set('lazy', $update->createNamedParameter($lazy, IQueryBuilder::PARAM_BOOL))
->set('lazy', $update->createNamedParameter(($lazy) ? 1 : 0, IQueryBuilder::PARAM_INT))
->set('type', $update->createNamedParameter($type, IQueryBuilder::PARAM_INT))
->where($update->expr()->eq('appid', $update->createNamedParameter($app)))
->andWhere($update->expr()->eq('configkey', $update->createNamedParameter($key)));
Expand Down Expand Up @@ -925,7 +928,7 @@ public function updateLazy(string $app, string $key, bool $lazy): bool {

$update = $this->connection->getQueryBuilder();
$update->update('appconfig')
->set('lazy', $update->createNamedParameter($lazy, IQueryBuilder::PARAM_BOOL))
->set('lazy', $update->createNamedParameter($lazy ? 1 : 0, IQueryBuilder::PARAM_INT))
->where($update->expr()->eq('appid', $update->createNamedParameter($app)))
->andWhere($update->expr()->eq('configkey', $update->createNamedParameter($key)));
$update->executeStatement();
Expand Down Expand Up @@ -1157,21 +1160,13 @@ private function loadConfig(?bool $lazy = false): void {
if (!$this->migrationCompleted) {
$qb->select('appid', 'configkey', 'configvalue');
} else {
$qb->select('appid', 'configkey', 'configvalue', 'type', 'lazy');
// we only need value from lazy when loadConfig does not specify it
$qb->select('appid', 'configkey', 'configvalue', 'type');

if ($lazy !== null) {
if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE) {
// Oracle does not like empty string nor false boolean !?
if ($lazy) {
$qb->where($qb->expr()->eq('lazy', $qb->createNamedParameter('1', IQueryBuilder::PARAM_INT)));
} else {
$qb->where($qb->expr()->orX(
$qb->expr()->isNull('lazy'),
$qb->expr()->eq('lazy', $qb->createNamedParameter('0', IQueryBuilder::PARAM_INT))
));
}
} else {
$qb->where($qb->expr()->eq('lazy', $qb->createNamedParameter($lazy, IQueryBuilder::PARAM_BOOL)));
}
$qb->where($qb->expr()->eq('lazy', $qb->createNamedParameter($lazy ? 1 : 0, IQueryBuilder::PARAM_INT)));
} else {
$qb->addSelect('lazy');
}
}

Expand All @@ -1195,9 +1190,8 @@ private function loadConfig(?bool $lazy = false): void {

$rows = $result->fetchAll();
foreach ($rows as $row) {
// if migration is not completed, 'lazy' and 'type' does not exist in $row
// also on oracle, lazy can be null ...
if ($row['lazy'] ?? false) {
// most of the time, 'lazy' is not in the select because its value is already known
if (($row['lazy'] ?? ($lazy ?? 0) ? 1 : 0) === 1) {
$cache = &$this->lazyCache;
} else {
$cache = &$this->fastCache;
Expand Down Expand Up @@ -1282,6 +1276,15 @@ public function getValue($app, $key, $default = null) {
* @deprecated
*/
public function setValue($app, $key, $value) {
/**
* TODO: would it be overkill, or decently improve performance, to catch
* call to this method with $key='enabled' and 'hide' config value related
* to $app when the app is disabled (by modifying entry in database: lazy=lazy+2)
* or enabled (lazy=lazy-2)
*
* this solution would remove the loading of config values from disabled app
* unless calling the method {@see loadConfigAll()}
*/
return $this->setTypedValue($app, $key, (string)$value, false, self::VALUE_MIXED);
}

Expand Down
2 changes: 1 addition & 1 deletion version.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
// between betas, final and RCs. This is _not_ the public version number. Reset minor/patch level
// when updating major/minor version number.

$OC_Version = [29, 0, 0, 4];
$OC_Version = [29, 0, 0, 5];

// The human-readable string
$OC_VersionString = '29.0.0 dev';
Expand Down
Loading