Repository navigation
Conversation
85fef0e to
cc81c15
Compare
d60bcda to
1008fcf
Compare
e87f653 to
30f1438
Compare
|
I rebased the series on current main and fixed some selectize-related bugs (unnecessary escaping, some options were missing from patch forms selectize, it also was impossible to type after selecting an item from the dropdown). I'm planning to make the following changes, likely by Friday:
|
57b25ec to
42eb863
Compare
This will allow us to store color codes cleanly in the database.
42eb863 to
8ea27f5
Compare
Labels are arbitrary bits of metadata attached to a submission. They can be used to signify priority, category, or other similar information. They can also be used to filter patches and identify the ones most interesting to a given user. Labels can be associated with a project to ensure that one project can use a totally different set of labels to another and to, in the future, allow a project administrator to use their own labels. However, they can also be global, which is useful for things that would be common across multiple projects such as "RFC".
There are two added: a general labels view that includes both project and non-project labels, and an inline labels view that's part of the project.
This is required to ensure we can filter delegates by project. If we don't do this, our <select> gets stupid long, as seen in commit 198139e.
Nothing too complicated here. The forms we have for this need some serious cleanup/beautification, but that's a problem for another day. Modify create-bundle style to match Selectize style. Remove redundant create-bundle style.
REST: Fix bug 335 tests One extra query is necessary for patch/cover endpoints to fetch labels information.
The filter allows filtering by project-specific and global labels. It also supports negative filtering. Prepending a label name with a minus sign excludes patches with that label from search results.
Patches in the API can be filtered by labels using the 'labels' query parameter.
a741c95 upgraded selectize to 0.13.5. However, it looks like a non-standalone version was uploaded by accident. On page load, the following error can be seen in debug console: "Selectize: Dependency MicroPlugin is missing". Because of that, the submitter autocomplete no longer works. Version 0.13.5 was not released on Github, so I have decided to upgrade the library to the latest available version. Add updated files to .gitattributes to skip them in patches.
b53bf9f to
f7f65ca
Compare
The command re-parses subject headers of patches from given projects and adds label information to the database. The main usecase is to refresh label information on existing patches after adding a new label to the project.
Plugin refocuses text input of a multiselect after an item from the dropdown is selected. Move all selectize plugins to a separate file. Use the plugin in all labels fields.
Labels parsed out of a cover letter are automatically assigned to all patches in the series, even if the cover letter comes in late.
help_text is not relevant to database migrations. This commit modifies the label migration instead of creating a new migration.
f7f65ca to
32a6573
Compare
unit.test_series.SeriesDependencyTestCase.test_dependency_multi_1 sometimes fails on CI in py314-django52 environment with Postgres. I cannot reproduce this locally, but the issue could be related to unspecified ordering.
|
I gave up on the JSON version, I think it makes no sense if many database engines are supported. GIN indexes in Postgres work well and are supported by Django. Postgres also supports array datatype, which is better suited for this task. Multi-valued indexes in MySQL have some limitations and are not officially supported by Django (although the database sill uses them in ordinary queries). The latest, mostly complete version of my JSON attempt is on labels_json_str branch of my fork. MySQL multi-valued index issuesThe main issue I have with the MySQL multi-valued index is that rows with empty arrays are not added to the index at all. This becomes a problem with multi-column indexes (the idea was to have another ~covering index, ex. project_id, archived, state_id, labels...) as the database may use such index even when the query does not filter by labels. In that case, all rows with an empty list of labels will be skipped from the results. Rows with SQL NULLs (but not JSON nulls, those cause an error) are supported by the index. So technically, it is possible to work around this problem with a custom field that saves empty arrays as SQL NULL. I find this workaround kind of hacky.In the end, I have managed to optimize the filtering query further, to the point where performance is comparable with my JSON attempts. Indexes introduced with recent query optimization patches really help (thanks for adding the primary key to the new patches index). If more optimizations are needed, it's probably be best to just cache commonly queried Patch fields in the PatchLabels table, with an index on that. Benchmark resultsThe benchmark was done in the default view (not archived, action required) on a project with 300k patches and 5 labels. The instance stores 2 million patches. The first label is assigned to roughly 50% of the patches, the second to ~25% and so on. The test involved filtering by various combinations of the labels (first, first and second..., all labels, first label and not the second label, first, second but not third...) and traversing up to 200 pages of the results.MySQL average: 701 ms (m2m) vs 652 ms (json) Both methods perform similarly when filtering by a single label (349 ms for JSON vs 449 ms on Postgres, 648 ms for JSON vs 679 ms on MySQL), for m2m performance gets worse the more labels are included in the filter. With JSON on Postgres it's the opposite, which is not surprising as the GIN index is optimized for overlap filtering. I have also added some features I find optional:
|
This series is a continuation of labels series published by Stephen Finucane
back in 2018.
Changes since v2:
I have started implementing series -> patch labels inheritance. It's not
included in this series as I'm not sure how it should work. As I understand
it, the name of the series may be changed as new patches are parsed.
I'm also not sure where in the UI users could see/modify labels for series.
This version uses a many-to-many relation for storing labels assigned to a
patch, like in v2. I understand that there is a performance concern as this
could cause excessive JOINs in queries. From what I can see, Django only uses
JOINs when patches are filtered by labels. When patches are just listed without
any filters, a separate query to the patch-labels table is executed instead
of a JOIN, that query seems to be rather cheap.
I have a POC alternative version that uses JSON columns to store an array with
labels inside the patches table.
So far it's not obvious to me that the JSON version is better performance-wise
(in my tests it sometimes was, sometimes not, especially on MySQL, that could be
an issue with my setup or the tests). Downsides: the code is more complex,
and I think there may be some issues around update atomicity.
Some of the commits from v2 were modified beyond just rebase and formatting.
I have retained the original author, unless the changes were really significant.
I'm submitting these patches now, I have added myself to Signed-off-by.
I'm not sure if this is 100% OK, so please let me know.
Sponsored-By: The Linux Foundation
Closes: #22