Thread: Removing the TRACE_SORT macro

Currently, debug instrumentation for sorting is only available if the
TRACE_SORT macro was defined when PostgreSQL was compiled. It is
defined by default, and so in practice it's always available; there is
really no upside to disabling it. "18.17. Developer Options" notes
this restriction for trace_sort, which is the only such restriction.

The number of TRACE_SORT elog() logging callers has grown
significantly in the past couple of releases. The associated "#ifdef
TRACE_SORT" crud has also grown.

I propose that we completely remove the TRACE_SORT macro, and all the
associated crud. Just having that as an option suggests that there is
some possible upside to disabling trace_sort, which is clearly not
true. I will write a patch doing this if there are no objections. I
think this is justifiable as clean-up for 9.6.

-- 
Peter Geoghegan

On Mon, Apr 11, 2016 at 2:35 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> Yeh, sort has changed enough now that fixes weren't going to backpatch
> cleanly, so its a good time to do cleanup.

I wonder if the category of "Developer Options" is appropriate for
trace_sort. trace_sort is closer to log_executor_stats, which is
categorized as "Statistics / Monitoring". I guess it isn't worth
changing now, though.

-- 
Peter Geoghegan

On Mon, Apr 11, 2016 at 4:43 PM, Peter Geoghegan <pg@heroku.com> wrote:
> Currently, debug instrumentation for sorting is only available if the
> TRACE_SORT macro was defined when PostgreSQL was compiled. It is
> defined by default, and so in practice it's always available; there is
> really no upside to disabling it. "18.17. Developer Options" notes
> this restriction for trace_sort, which is the only such restriction.
>
> The number of TRACE_SORT elog() logging callers has grown
> significantly in the past couple of releases. The associated "#ifdef
> TRACE_SORT" crud has also grown.
>
> I propose that we completely remove the TRACE_SORT macro, and all the
> associated crud. Just having that as an option suggests that there is
> some possible upside to disabling trace_sort, which is clearly not
> true. I will write a patch doing this if there are no objections. I
> think this is justifiable as clean-up for 9.6.

I'm not excited about this change.  It doesn't really buy us anything
that I can see.  For one thing, I think we've arguably got too much
trace_sort output from sorts now and should look to reduce that
instead of further normalizing it.  For another thing, the only
advantage to removing it is that it makes it easier to add even more,
and we're not going to do that in 9.6.  If we decide to do it in 9.7,
we can consider the proposal to remove TRACE_SORT calls then.  There
is just nothing urgent about this.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon, Apr 11, 2016 at 2:54 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> I'm not excited about this change.  It doesn't really buy us anything
> that I can see.  For one thing, I think we've arguably got too much
> trace_sort output from sorts now and should look to reduce that
> instead of further normalizing it.  For another thing, the only
> advantage to removing it is that it makes it easier to add even more,
> and we're not going to do that in 9.6.  If we decide to do it in 9.7,
> we can consider the proposal to remove TRACE_SORT calls then.  There
> is just nothing urgent about this.

Of course it isn't urgent. I seem to have significant difficulty
getting proposals accepted that are not urgent, even if they are
important. While this proposal isn't important, it also isn't hard or
in any way risky or time consuming. I just want to get rid of the
TRACE_SORT crud. That's all.

I'll put it on my long-term TODO list.

-- 
Peter Geoghegan