Horton

Thread

Commits

  1. Fix deadlock hazard in CREATE INDEX CONCURRENTLY

  1. Deadlock between concurrent index builds on different tables

    Jeremy Finzel <finzelj@gmail.com> — 2017-12-22T21:53:25Z

    I am attempting to build several indexes in parallel, guaranteeing that I
never build one on the same table twice.  I understand I can't build two on
the same table at once or I will get a deadlock.  However, I am also
getting a deadlock when doing several in parallel on different tables.

Here is an example of the error I am getting:

2017-12-22 15:48:07.669 CST,"CREATE INDEX",2017-12-22 15:48:02
CST,8/32,0,ERROR,40P01,"deadlock detected","Process 4470 waits for
ShareLock on virtual transaction 4/262; blocked by process 4466.
Process 4466 waits for ShareLock on virtual transaction 8/32; blocked by
process 4470.
Process 4470: CREATE INDEX CONCURRENTLY index_foo_on_created_at ON foo
USING btree (created_at);
Process 4466: CREATE INDEX CONCURRENTLY index_bar_on_id ON bar USING btree
(id);","See server log for query details.",,,,"CREATE INDEX CONCURRENTLY
index_foo_on_created_at ON foo USING btree (created_at);",,,""

Here is my process:

   - Kick off one index build and background 1 second apart, using a queue
   table to determine what indexes to build and what is finished
   - When I determine if a index can be built, I first check if there is
   another already in build on target table before starting another
   - After the index is built in each workflow, I mark it as built in a
   queue table

I assume that one of the indexes (the earlier transaction) is building and
taking so long that several finished indexes end up waiting, and there is
perhaps a limit to that?

Any insight appreciated.

Thanks,
Jeremy

  2. Re: Deadlock between concurrent index builds on different tables

    Alban Hertroys <haramrae@gmail.com> — 2017-12-23T11:08:42Z

    > On 22 Dec 2017, at 22:53, Jeremy Finzel <finzelj@gmail.com> wrote:
> 
> 2017-12-22 15:48:07.669 CST,"CREATE INDEX",2017-12-22 15:48:02 CST,8/32,0,ERROR,40P01,"deadlock detected","Process 4470 waits for ShareLock on virtual transaction 4/262; blocked by process 4466.
> Process 4466 waits for ShareLock on virtual transaction 8/32; blocked by process 4470.
> Process 4470: CREATE INDEX CONCURRENTLY index_foo_on_created_at ON foo USING btree (created_at);
> Process 4466: CREATE INDEX CONCURRENTLY index_bar_on_id ON bar USING btree (id);","See server log for query details.",,,,"CREATE INDEX CONCURRENTLY index_foo_on_created_at ON foo USING btree (created_at);",,,""

If I read these logs correctly, you are creating the exact same index on foo (created_at) in both processes, which is just what you were trying to prevent doing.

Alban Hertroys
--
If you can't see the forest for the trees,
cut the trees and you'll find there is no forest.

  3. Re: Deadlock between concurrent index builds on different tables

    Martin Marques <martin.marques@2ndquadrant.com> — 2017-12-23T11:22:33Z

    El 23/12/17 a las 08:08, Alban Hertroys escribió:
> 
>> On 22 Dec 2017, at 22:53, Jeremy Finzel <finzelj@gmail.com> wrote:
>>
>> 2017-12-22 15:48:07.669 CST,"CREATE INDEX",2017-12-22 15:48:02 CST,8/32,0,ERROR,40P01,"deadlock detected","Process 4470 waits for ShareLock on virtual transaction 4/262; blocked by process 4466.
>> Process 4466 waits for ShareLock on virtual transaction 8/32; blocked by process 4470.
>> Process 4470: CREATE INDEX CONCURRENTLY index_foo_on_created_at ON foo USING btree (created_at);
>> Process 4466: CREATE INDEX CONCURRENTLY index_bar_on_id ON bar USING btree (id);","See server log for query details.",,,,"CREATE INDEX CONCURRENTLY index_foo_on_created_at ON foo USING btree (created_at);",,,""
> 
> If I read these logs correctly, you are creating the exact same index on foo (created_at) in both processes, which is just what you were trying to prevent doing.

They are two distinct indexes. One on foo(create_at) and the other on
bar(id)


-- 
Martín Marqués                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

  4. Re: Deadlock between concurrent index builds on different tables

    Martin Marques <martin.marques@2ndquadrant.com> — 2017-12-23T11:40:18Z

    El 22/12/17 a las 18:53, Jeremy Finzel escribió:
> I am attempting to build several indexes in parallel, guaranteeing that
> I never build one on the same table twice.  I understand I can't build
> two on the same table at once or I will get a deadlock.  However, I am
> also getting a deadlock when doing several in parallel on different tables.

On the same table you wouldn't get a dead lock as CREATE INDEX
CONCURRENTLY takes a "share update exclusive lock" which is incompatible
with vacuum and schema changes (and stronger locks), and a second CREATE
INDEX CONCURRENTLY would be another schema change.

So the first one would lock the second, which will have to wait until
completion of the locking statement.

> Here is an example of the error I am getting:
> 
> 2017-12-22 15:48:07.669 CST,"CREATE INDEX",2017-12-22 15:48:02
> CST,8/32,0,ERROR,40P01,"deadlock detected","Process 4470 waits for
> ShareLock on virtual transaction 4/262; blocked by process 4466.
> Process 4466 waits for ShareLock on virtual transaction 8/32; blocked by
> process 4470.
> Process 4470: CREATE INDEX CONCURRENTLY index_foo_on_created_at ON foo
> USING btree (created_at);
> Process 4466: CREATE INDEX CONCURRENTLY index_bar_on_id ON bar USING
> btree (id);","See server log for query details.",,,,"CREATE INDEX
> CONCURRENTLY index_foo_on_created_at ON foo USING btree (created_at);",,,""

The only thing I can think of is that there's a foreign key from foo to
bar(id), but the create index on bar shouldn't prevent a share lock on
foo, even if such a restriction exists.

-- 
Martín Marqués                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

  5. Re: Deadlock between concurrent index builds on different tables

    Jeff Janes <jeff.janes@gmail.com> — 2017-12-23T16:59:38Z

    On Fri, Dec 22, 2017 at 1:53 PM, Jeremy Finzel <finzelj@gmail.com> wrote:

> I am attempting to build several indexes in parallel, guaranteeing that I
> never build one on the same table twice.  I understand I can't build two on
> the same table at once or I will get a deadlock.  However, I am also
> getting a deadlock when doing several in parallel on different tables.
>
> Here is an example of the error I am getting:
>
> 2017-12-22 15:48:07.669 CST,"CREATE INDEX",2017-12-22 15:48:02
> CST,8/32,0,ERROR,40P01,"deadlock detected","Process 4470 waits for
> ShareLock on virtual transaction 4/262; blocked by process 4466.
> Process 4466 waits for ShareLock on virtual transaction 8/32; blocked by
> process 4470.
> Process 4470: CREATE INDEX CONCURRENTLY index_foo_on_created_at ON foo
> USING btree (created_at);
> Process 4466: CREATE INDEX CONCURRENTLY index_bar_on_id ON bar USING btree
> (id);","See server log for query details.",,,,"CREATE INDEX CONCURRENTLY
> index_foo_on_created_at ON foo USING btree (created_at);",,,""
>
> Here is my process:
>
>    - Kick off one index build and background 1 second apart, using a
>    queue table to determine what indexes to build and what is finished
>    - When I determine if a index can be built, I first check if there is
>    another already in build on target table before starting another
>    - After the index is built in each workflow, I mark it as built in a
>    queue table
>
> I assume that one of the indexes (the earlier transaction) is building and
> taking so long that several finished indexes end up waiting, and there is
> perhaps a limit to that?
>

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.

Cheers,

Jeff

  6. Re: Deadlock between concurrent index builds on different tables

    Jeremy Finzel <finzelj@gmail.com> — 2017-12-23T18:11:47Z

    >
>
>> 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.
>
> Cheers,
>
> Jeff
>
 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.

So if I start 2 index builds within a second of each other, you are saying
that the first will not finish even though it started first because there
is a long transaction from the second index build? I would expect much more
deadlocks in this case, but building 5 indexes concurrently at a time of a
total 15 indexes I only get one or two deadlocks. I would expect many more
if I can’t even have 2 at once...

Thanks,
Jeremy

  7. Re: Deadlock between concurrent index builds on different tables

    Jeremy Finzel <finzelj@gmail.com> — 2017-12-23T18:13:20Z

    >
> The only thing I can think of is that there's a foreign key from foo to
> bar(id), but the create index on bar shouldn't prevent a share lock on
> foo, even if such a restriction exists.
>
> --
> Martín Marqués                http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>
No, there are no foreign keys at all in this db. It is a logical replica.

Thanks,
Jeremy

  8. Re: Deadlock between concurrent index builds on different tables

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2017-12-26T16:28:34Z

    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

  9. Re: Deadlock between concurrent index builds on different tables

    Jeremy Finzel <finzelj@gmail.com> — 2017-12-26T17:00:55Z

    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

  10. Re: Deadlock between concurrent index builds on different tables

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2017-12-26T17:11:52Z

    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

  11. Re: Deadlock between concurrent index builds on different tables

    Alban Hertroys <haramrae@gmail.com> — 2017-12-26T23:27:05Z

    > 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 test out 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.

  12. Re: Deadlock between concurrent index builds on different tables

    Michael Paquier <michael.paquier@gmail.com> — 2017-12-27T00:43:03Z

    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 test out 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

  13. Re: Deadlock between concurrent index builds on different tables

    Jeremy Finzel <finzelj@gmail.com> — 2017-12-27T03:03:18Z

    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

  14. Re: Deadlock between concurrent index builds on different tables

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2017-12-27T13:56:20Z

    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

  15. Re: Deadlock between concurrent index builds on different tables

    Jeff Janes <jeff.janes@gmail.com> — 2018-01-03T18:43:44Z

    On Tue, Dec 26, 2017 at 10:03 PM, Jeremy Finzel <finzelj@gmail.com> wrote:

>
>
> 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!
>

Hi Jeremy,

I don't know if you were following the discussion over on the hackers list,
but Alvaro has committed the fix and it should be in the 9.6.7 version when
that gets released.

Cheers,

Jeff