Re: Is WAL_DEBUG related code still relevant today? - Mailing list pgsql-hackers
From | Bharath Rupireddy |
---|---|
Subject | Re: Is WAL_DEBUG related code still relevant today? |
Date | |
Msg-id | CALj2ACU1cDO37Cd+DSciRiBA3STwnbsBTWX+pgK3RCgmzmj0kA@mail.gmail.com Whole thread Raw |
In response to | Re: Is WAL_DEBUG related code still relevant today? (Nathan Bossart <nathandbossart@gmail.com>) |
Responses |
Re: Is WAL_DEBUG related code still relevant today?
|
List | pgsql-hackers |
On Sun, Dec 3, 2023 at 4:00 AM Nathan Bossart <nathandbossart@gmail.com> wrote: > > On Sat, Dec 02, 2023 at 07:36:29PM +0530, Bharath Rupireddy wrote: > > I started to think if this code is needed at all in production. How > > about we do either of the following? > > Well, the fact that this code is hidden behind an off-by-default macro > seems like a pretty strong indicator that it is not intended for > production. But that doesn't mean we should remove it. I think all that the WAL_DEBUG code offers can be achieved with other means after adjusting a few pieces. Please see the commit message in the v1 patch posted in this thread at https://www.postgresql.org/message-id/CALj2ACW5zPMT09eqXvh2Ve7Kz02HShTwyjG%2BxTzkrzeKtQMnQQ%40mail.gmail.com. > > a) Remove the WAL_DEBUG macro and move all the code under the > > wal_debug GUC? Since the GUC is already marked as DEVELOPER_OPTION, > > the users will know the consequences of enabling it in production. > > I think the key to this option is verifying there's no measurable > performance impact. FWIW, enabling this has a huge impact in production. For instance, recovery TAP tests are ~10% slower with the WAL_DEBUG macro enabled. I don't think we go the route of keeping this code. WAL_DEBUG macro enabled: All tests successful. Files=38, Tests=531, 157 wallclock secs ( 0.18 usr 0.05 sys + 14.96 cusr 16.11 csys = 31.30 CPU) Result: PASS HEAD: All tests successful. Files=38, Tests=531, 143 wallclock secs ( 0.15 usr 0.06 sys + 14.24 cusr 15.62 csys = 30.07 CPU) Result: PASS > > b) Remove both the WAL_DEBUG macro and the wal_debug GUC. I don't > > think (2) is needed to be in core especially when tools like > > pg_walinspect and pg_waldump can do the same job. And, the messages in > > (3) and (4) can be turned to some DEBUGX level without being under the > > WAL_DEBUG macro. > > Is there anything provided by wal_debug that can't be found via > pg_walinspect/pg_waldump? I don't think so. The WAL record decoding can be achieved with pg_walinspect or pg_waldump. The page comparison check in generic_xlog.c can be moved under USE_ASSERT_CHECKING macro. PSA v1 patch posted in this thread. > > I have no idea if anyone uses WAL_DEBUG macro and wal_debug GUCs in > > production, if we have somebody using it, I think we need to fix the > > compilation and test failure issues, and start testing this code > > (perhaps I can think of setting up a buildfarm member to help here). > > +1 for at least fixing the code and tests, provided we decide to keep it. With no real use of this code in production, instead of fixing compilation issues and TAP test failures to maintain the code, I think it's better to adjust a few pieces and remove the other stuff like in the attached v1 patch. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
pgsql-hackers by date: