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

Conversation

ArtificialOwl
Copy link
Member

@ArtificialOwl ArtificialOwl commented Jan 18, 2024

follow-up of #42820

  • only create new fields if not existing yet
  • removing appconfig_configkey_key index on single field configkey as there is no search on this field
  • redefine lazy as Types::INTEGER (smallint(6)) instead of Types::BOOLEAN to be able to set it a NotNull (set in 28-rc2)
  • index on lazy
  • removing the use of $migrationCompleted that was needed during the migration from 28 to 29 but not anymore since we generate the (previously) missing fields lazy and type in 28
  • do not select lazy during loadConfig as we already know its value

Note: After some discussion, it seems better to switch lazy to INTEGER so the field can be set as NotNull (because Oracle)

Fixes #43103

@ArtificialOwl ArtificialOwl force-pushed the enh/noid/improve-index-on-appconfig branch 2 times, most recently from 3cd94e9 to bbe1bee Compare January 22, 2024 19:15
@ArtificialOwl ArtificialOwl changed the title [wip] improve index on appconfig improve index on appconfig Jan 22, 2024
@ArtificialOwl ArtificialOwl added this to the Nextcloud 29 milestone Jan 22, 2024
@ArtificialOwl ArtificialOwl requested review from artonge, a team, icewind1991 and Altahrim and removed request for a team January 22, 2024 19:20
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/improve-index-on-appconfig branch from bbe1bee to d300919 Compare January 23, 2024 13:03
@ArtificialOwl ArtificialOwl requested review from nickvergessen and removed request for nickvergessen January 23, 2024 13:12
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/improve-index-on-appconfig branch 2 times, most recently from 6615418 to 5ad0674 Compare January 23, 2024 16:35
@ArtificialOwl ArtificialOwl force-pushed the enh/noid/improve-index-on-appconfig branch from 5ad0674 to 3163277 Compare January 24, 2024 20:05
@ArtificialOwl
Copy link
Member Author

ArtificialOwl commented Jan 24, 2024

I have estimated that a full squash+rebase was a lot better as I reverted a lot of code:

  • upstream of [stable28] prepare migration to lazy config #42820 to master
  • commenting most of the code from previous migration step Version28000Date20231126110901 (+ explication)
  • commenting most of the code from previous migration step Version28000Date20231126110901 (+ explication)
  • new migration step Version29000Date20240124132201 to remove previous boolean lazy field and related indexes, fix type to unsigned and remove useless index on configkey.
  • new migration step Version29000Date20240124132202 to create a smallint lazy field.
  • keep migration fall back on reading app config values to keep upgrade compatibility with versions prior to 28.0.2
  • switching write/read request to lazy field from managing a boolean to managing a smallint
  • remove lazy from select list if known when calling loadConfig()

tested on

  • fresh install
  • upgrade from 28.0.2-rc2
  • upgrade from 28.0.1 (only issue doing this is that writing (new/edit) app config value will throw sql exception

final result on each test:

MariaDB [nextcloud_nc29]> DESCRIBE oc_appconfig; SHOW INDEX FROM oc_appconfig;
+-------------+----------------------+------+-----+---------+-------+
| Field       | Type                 | Null | Key | Default | Extra |
+-------------+----------------------+------+-----+---------+-------+
| appid       | varchar(32)          | NO   | PRI |         |       |
| configkey   | varchar(64)          | NO   | PRI |         |       |
| configvalue | longtext             | YES  |     | NULL    |       |
| type        | int(10) unsigned     | NO   |     | 2       |       |
| lazy        | smallint(5) unsigned | NO   | MUL | 0       |       |
+-------------+----------------------+------+-----+---------+-------+
5 rows in set (0.002 sec)

+--------------+------------+-----------+--------------+-------------+-----------+-------------+----------+--------+------+------------+---------+---------------+---------+
| Table        | Non_unique | Key_name  | Seq_in_index | Column_name | Collation | Cardinality | Sub_part | Packed | Null | Index_type | Comment | Index_comment | Ignored |
+--------------+------------+-----------+--------------+-------------+-----------+-------------+----------+--------+------+------------+---------+---------------+---------+
| oc_appconfig |          0 | PRIMARY   |            1 | appid       | A         |         142 |     NULL | NULL   |      | BTREE      |         |               | NO      |
| oc_appconfig |          0 | PRIMARY   |            2 | configkey   | A         |         142 |     NULL | NULL   |      | BTREE      |         |               | NO      |
| oc_appconfig |          1 | ac_lazy_i |            1 | lazy        | A         |           2 |     NULL | NULL   |      | BTREE      |         |               | NO      |
+--------------+------------+-----------+--------------+-------------+-----------+-------------+----------+--------+------+------------+---------+---------------+---------+
3 rows in set (0.001 sec)

Signed-off-by: Maxence Lange <[email protected]>
ArtificialOwl and others added 2 commits January 25, 2024 10:33
Co-authored-by: Joas Schilling <[email protected]>
Signed-off-by: Maxence Lange <[email protected]>
Co-authored-by: Joas Schilling <[email protected]>
Signed-off-by: Maxence Lange <[email protected]>
@ArtificialOwl ArtificialOwl merged commit 08ec513 into master Jan 26, 2024
55 checks passed
@ArtificialOwl ArtificialOwl deleted the enh/noid/improve-index-on-appconfig branch January 26, 2024 10:53
@blizzz blizzz mentioned this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Redundant index appconfig.ac_app_lazy_i
3 participants