On 8/10/2013 10:29 PM, Eli Carter wrote:
> Ok, I've updated the patch series:
> 
> part1: implicit sync

LGTM.

> 
> part2: warn on slow SQL
>     Unmodified from the initial post; when debug_sql is enabled, logs a
> warning if an SQL statement takes over 10 seconds to execute.

Wouldn't it be better to fetch the value of "10" seconds from the
configuration? ([trac] debug_sql_timing_threshold?). 3 seconds may be
already too long for many people...

> 
> part3: uncache previous_rev()
>     Makes previous_rev() unconditionally go to the repository without
> hitting the node_change table.  (Was "part3a".)

I would prefer that you keep the previous code, but commented out, and
the comment saying in addition "commented out for now", making it clear
this is a workaround for the way the SQL query is currently implemented.
That clearly shows that we tried to use the cache, should anyone want to
improve speed by using the cache again.

> 
> part4: warn on slow next_rev()
>     This logs a warning if next_rev() takes more than 10 seconds to
> complete, suggesting enabling [trac] debug_sql.  (Was "part3b".)
>     The purpose of this is to give the admin that first breadcrumb to
> lead them to the underlying cause of their slowness.
> 

Same thing, we could use the configured threshold for the timing.
Default could still be 10s of course, but people could then easily make
it more sensitive without hacking the code.

(And we could add another setting for timing the requests themselves,
that would be useful as well.)

Lastly, what do you think about the suggestion of turning the "next
revision" link into a relative one, in the code browser page?

-- Christian

-- 
You received this message because you are subscribed to the Google Groups "Trac 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to