Thread: Re: Deadlock between concurrent index builds on different tables
Jeremy Finzel wrote: > > > > > >> Each index build needs to wait for all other transactions > >> (Including the ones used by the other index build) to finish. > >> So I don't think a deadlock here is unexpected. > Does that mean I should never build more than one concurrent index at > a time within the entire cluster? If so, that is not clear from the > documentation. No, there is no such expectation. Jeff analyzed your scenario, discovered a bug and sent a patch to fix it -- care to test it and report back? You can get it from here: https://www.postgresql.org/message-id/CAMkU=1ztk3TpQdcUNbxq93pc80FrXUjpDWLGMeVBDx71GHNwZQ@mail.gmail.com -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Dec 26, 2017 at 10:28 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
Jeremy Finzel wrote:
> >
> >
> >> Each index build needs to wait for all other transactions
> >> (Including the ones used by the other index build) to finish.
> >> So I don't think a deadlock here is unexpected.
> Does that mean I should never build more than one concurrent index at
> a time within the entire cluster? If so, that is not clear from the
> documentation.
No, there is no such expectation. Jeff analyzed your scenario,
discovered a bug and sent a patch to fix it -- care to test it and
report back? You can get it from here:
https://www.postgresql.org/message-id/CAMkU= 1ztk3TpQdcUNbxq93pc80FrXUjpDWL GMeVBDx71GHNwZQ@mail.gmail.com
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
I would be thrilled to review it, but I need a little direction as I have not done a patch review before. I have been reading through some of the developer FAQ and patch info. The attached file is simply a git diff, so I'm not sure how I am to use this? Is there a specific source version I can download and compile? I know where to get the current master, etc., from git, but where can I get the patched version or what is the proper way to apply the patch to current master?
Thanks!
Jeremy
Jeremy Finzel wrote: > On Tue, Dec 26, 2017 at 10:28 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> > wrote: > > > Jeremy Finzel wrote: > > > > > > > >> Each index build needs to wait for all other transactions > > > >> (Including the ones used by the other index build) to finish. > > > >> So I don't think a deadlock here is unexpected. > > > > > Does that mean I should never build more than one concurrent index at > > > a time within the entire cluster? If so, that is not clear from the > > > documentation. > > > > No, there is no such expectation. Jeff analyzed your scenario, > > discovered a bug and sent a patch to fix it -- care to test it and > > report back? You can get it from here: > > > > https://www.postgresql.org/message-id/CAMkU=1ztk3TpQdcUNbxq93pc80FrXUjpDWL > > GMeVBDx71GHNwZQ@mail.gmail.com > I would be thrilled to review it, but I need a little direction as I have > not done a patch review before. I have been reading through some of the > developer FAQ and patch info. The attached file is simply a git diff, so > I'm not sure how I am to use this? Is there a specific source version I > can download and compile? I know where to get the current master, etc., > from git, but where can I get the patched version or what is the proper way > to apply the patch to current master? I don't think a patch review as such is necessary -- that code is very complex and you'd need to learn about a lot of internals (though I won't stop if you want to learn). I was thinking about testing it, instead. To create a patched build, 1. get a clone with the branch you're on. Assuming you're on 9.6, it'd be like this git clone <URL> -b REL9_6_STABLE 2. apply the patch on top cd postgresql patch -p1 < /path/to/file.diff # you could use "git apply" instead (or "git am", but not with this one) 3. configure and make ./configure <configure options> make make install 4. run it initdb -D <somedir> # to create a fresh datadir pg_ctl <whatever> You may need additional packages (zlib devel, readline devel, others; see https://www.postgresql.org/docs/9.6/static/installation.html) For the options in step 3 you could use whatever your current server has; use "pg_config --configure" to find these out. You're gonna need same flags if you want to use your existing data directory. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
> On 26 Dec 2017, at 18:11, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: … > 3. configure and make > ./configure <configure options> > make > make install … > For the options in step 3 you could use whatever your current server > has; use "pg_config --configure" to find these out. You're gonna need > same flags if you want to use your existing data directory. Does that mean that at step 3 one could issue this?: ./configure `pg_config —configure` If I had the sources at hand, I'd try that myself, but I don't and getting those is frankly a bit of a hassle to just testout whether that works ;) Alban Hertroys -- If you can't see the forest for the trees, cut the trees and you'll find there is no forest.
On Wed, Dec 27, 2017 at 12:27:05AM +0100, Alban Hertroys wrote: > >> On 26 Dec 2017, at 18:11, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > > … > > > 3. configure and make > > ./configure <configure options> > > make > > make install > > … > > > For the options in step 3 you could use whatever your current server > > has; use "pg_config --configure" to find these out. You're gonna need > > same flags if you want to use your existing data directory. > > Does that mean that at step 3 one could issue this?: > > ./configure `pg_config —configure` > > If I had the sources at hand, I'd try that myself, but I don't and getting those is frankly a bit of a hassle to just testout whether that works ;) For this issue I don't think that you are going to care much about dependencies with low-level libraries like SSL or such as the behavior is in integrality linked with PostgreSQL internals and the physical representation of how transactions are handled with system catalogs. In short there is no need to be fancy :) -- Michael
Attachment
On Tue, Dec 26, 2017 at 11:11 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
Jeremy Finzel wrote:
> On Tue, Dec 26, 2017 at 10:28 AM, Alvaro Herrera <alvherre@alvh.no-ip.org>
> wrote:
>
> > Jeremy Finzel wrote:
> > > >
> > > >> Each index build needs to wait for all other transactions
> > > >> (Including the ones used by the other index build) to finish.
> > > >> So I don't think a deadlock here is unexpected.
> >
> > > Does that mean I should never build more than one concurrent index at
> > > a time within the entire cluster? If so, that is not clear from the
> > > documentation.
> >
> > No, there is no such expectation. Jeff analyzed your scenario,
> > discovered a bug and sent a patch to fix it -- care to test it and
> > report back? You can get it from here:
> >
> > https://www.postgresql.org/message-id/CAMkU= 1ztk3TpQdcUNbxq93pc80FrXUjpDWL
> > GMeVBDx71GHNwZQ@mail.gmail.com
> I would be thrilled to review it, but I need a little direction as I have
> not done a patch review before. I have been reading through some of the
> developer FAQ and patch info. The attached file is simply a git diff, so
> I'm not sure how I am to use this? Is there a specific source version I
> can download and compile? I know where to get the current master, etc.,
> from git, but where can I get the patched version or what is the proper way
> to apply the patch to current master?
I don't think a patch review as such is necessary -- that code is very
complex and you'd need to learn about a lot of internals (though I won't
stop if you want to learn). I was thinking about testing it, instead.
To create a patched build,
1. get a clone with the branch you're on. Assuming you're on 9.6, it'd
be like this
git clone <URL> -b REL9_6_STABLE
2. apply the patch on top
cd postgresql
patch -p1 < /path/to/file.diff
# you could use "git apply" instead (or "git am", but not with this one)
3. configure and make
./configure <configure options>
make
make install
4. run it
initdb -D <somedir> # to create a fresh datadir
pg_ctl <whatever>
You may need additional packages (zlib devel, readline devel, others;
see https://www.postgresql.org/docs/9.6/static/installation. html)
For the options in step 3 you could use whatever your current server
has; use "pg_config --configure" to find these out. You're gonna need
same flags if you want to use your existing data directory.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Many thanks for the great and simple explanation.
I was able to get this compiled, and ran the test before on stock 9.6.6, then on this patched version. I indeed reproduced it on 9.6.6, but on the patched version, it indeed fixes my issue!
I am indeed very interested in learning more about the whole patch review process, but I will have to save that for another day!
Let me know if you need me to check anything else!
Thanks,
Jeremy
Alban Hertroys wrote: > Does that mean that at step 3 one could issue this?: > > ./configure `pg_config —configure` Not exactly, because pg_config emits the arguments in quotes and the shell passes them as is to configure which doesn't like that. This works: eval ./configure `pg_config --configure` -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services