Ticket #4503 (assigned enhancement)

Opened 4 years ago

Last modified 2 years ago

[PATCH] performance issues

Reported by: mixedpuppy Assigned to: hasienda (accepted)
Priority: normal Component: TagsPlugin
Severity: major Keywords: performance
Cc: andref.dias@gmail.com, otaku42, rjollos Trac Release: 0.11

Description

The tags plugin is excellent, however, there are some pretty big performance issues.

One is, regardless of specifying a realm (via tags search page or a macro), all tag providers are asked for their tags, and only after that is the realm checked.

The second is checking permissions on each ticket (in the ticket provider). This is fine if you have a few hundred (or thousand) tickets. In a database with a couple hundred thousand tickets, the tags plugin becomes unusable. Relying on a ticket_view and tags_view at a "global" level is sufficient for the majority of cases.

The attached patch addresses both issues above, though I don't regard it as a "final" patch.

Attachments

tagsplugin-perf.patch (2.3 kB) - added by mixedpuppy on 01/23/09 15:10:40.
performance patch
lazy_permission_check.patch (1.5 kB) - added by otaku42 on 01/13/10 06:55:03.
Introduce new ini option that allows to skip "per ticket" permission checks

Change History

01/23/09 15:10:40 changed by mixedpuppy

  • attachment tagsplugin-perf.patch added.

performance patch

03/04/09 04:45:37 changed by andref.dias@gmail.com

  • cc set to andref.dias@gmail.com.
  • type changed from defect to enhancement.

These performance issues are of great interest to me too!

03/04/09 05:50:25 changed by athomas

The patch looks reasonable, though ideally the permission changes would be behind a configuration option - defaulting to the current behaviour, but configurable for a fast path.

(follow-up: ↓ 4 ) 01/12/10 11:53:03 changed by otaku42

  • owner changed from athomas to otaku42.

I just committed the first part of your patch (regarding realm) in r7377. Will try to come up with a patch for part two for the permission checks, where a configuration option modifies the behaviour (as athomas suggested above).

(in reply to: ↑ 3 ) 01/12/10 17:51:35 changed by otaku42

Replying to otaku42:

I just committed the first part of your patch (regarding realm) in r7377.

Guess I had a bad day:

I accidentally committed stuff to the tagged release v0.6 rather than to trunk, which of course was wrong. I'll thus roll back r7377 and re-apply it to trunk.

01/12/10 17:51:55 changed by otaku42

(In [7381]) In a query, a realm may be specified to limit results to the corresponding tag provider. So far all providers were polled anyway, which had a negative impact on performance. This is now fixed. Thanks to mixedpuppy for the patch. Refs #4503.

01/13/10 06:55:03 changed by otaku42

  • attachment lazy_permission_check.patch added.

Introduce new ini option that allows to skip "per ticket" permission checks

01/13/10 06:58:51 changed by otaku42

  • status changed from new to assigned.

lazy_permission_check.patch implements skipping per-ticket permissions as suggested by mixedpuppy, combined with an ini option that allows to enable this feature as suggested by athomas. The default is to stick to the old (current) behaviour.

Please review and test the patch. Once one confirms it works as intended, I'll commit it to the repository.

09/28/11 01:34:34 changed by hasienda

  • status changed from assigned to new.
  • cc changed from andref.dias@gmail.com to andref.dias@gmail.com, otaku42.
  • owner changed from otaku42 to hasienda.

Reading this ticket made me really sad.

There is a claim, but the reporter isn't able to test and provide feedback on an alternative fix within more than 1 1/2 years. No facts have been provided for these issues as well: "... fine if you have a few hundred (or thousand) tickets. In a database with a couple hundred thousand tickets, the tags plugin becomes unusable ..." - just rhetoric.

What are i.e. numbers and corresponding process/page load times? And why on earth nobody has been screaming for a pager? TracSearch and TracQuery have one. So TagsQuery will profit from getting one as well.

Again, step of with facts, results, please and don't discredit your report further by silence. In absence of such information I prepared a test environment with 30.000 test tickets. Man, this is dragging response time down, but only for formatting of results it seems:

  • 6766 of 30.000 tickets tagged with 'Test'
  • /tags?q=Test
  • query_results from db returned in 1,7 * 10-6 s
  • page with table view and 6767 rows rendered in ~ 50 s

Still I count on real application testing for upcoming features. Do you follow? Anyone?

10/04/11 00:16:47 changed by hasienda

  • status changed from new to assigned.

Permission checks are not an issue.

At least there is no measurable difference in my test environment, while the query logic might introduce remarkable slowdowns. And the Rendering does for sure, so a pager is a must-have to survive with several hundred or thousands of similarly tagged resources. Testing this right now.

The lazy permission check might get dropped in turn, if no one can step up with test results in favor of it. By now I'm unable to confirm it's effect with my test data. But I have no fine-grained per-resource restrictions applied here either, so this might mean nothing, perhaps.

10/05/11 22:55:06 changed by hasienda

(In [10737]) TagsPlugin: Wrap ListTagged results in Trac pager, refs #4503 and #9064.

This fixes SystemError: ../Objects/tupleobject.c ... occasionally seen with large sets of results. And response time is getting reasonable even for 30.000+ matching tagged resources.

Beware: Hold on, it's a feature-incomplete changeset - a step in transition.

10/05/11 23:04:32 changed by hasienda

(In [10738]) TagsPlugin: Move Genshi builder instructions to fragment template, refs #4503 and #9064.

Now all list formats and even the warning message about obsolete TagsQuery arguments are integrated into a HTML fragment template. Various goodies like differently shaded odd/even rows and 'no results message' have been added. Oddly squeezed ListTagged tables are gone as well - all due to re-using big parts of the TicketQuery code from Trac core.

10/06/11 00:46:40 changed by rjollos

  • cc changed from andref.dias@gmail.com, otaku42 to andref.dias@gmail.com, otaku42, rjollos.

10/15/11 00:06:39 changed by hasienda

(In [10781]) TagsPlugin: Add mincount functionality to main tags page, refs #4078 and #4503.

cloud_mincount allows to cut default cloud size and reduce cluttering and render time for the /tags page a bit when tagging is in heavy use. Calls of wiki macro TagCloud are not affected by this configuration option.

Add/Change #4503 ([PATCH] performance issues)




Change Properties
Action