Skip to content

Commit

Permalink
Fix index creation reporting, see #251.
Browse files Browse the repository at this point in the history 
The new option 'drop indexes' reuses the existing code to build all the
indexes in parallel but failed to properly account for that fact in the
summary report with timings.

While fixing this, also fix the SQL used to re-establish the indexes and
associated constraints to allow for parallel execution, the ALTER TABLE
statements would block in ACCESS EXCLUSIVE MODE otherwise and make our
efforts vain.
  • Loading branch information
@dimitri
dimitri committed Jul 18, 2015
1 parent 8511294 commit 54e2977
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 24 deletions.
6 changes: 5 additions & 1 deletion src/parsers/command-copy.lisp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@
(let* ((state-before (pgloader.utils:make-pgstate))
(summary (null *state*))
(*state* (or *state* (pgloader.utils:make-pgstate)))
(state-idx ,(when (getf options :drop-indexes)
`(pgloader.utils:make-pgstate)))
(state-after ,(when (or after (getf options :drop-indexes))
`(pgloader.utils:make-pgstate)))
,@(pgsql-connection-bindings pg-db-conn gucs)
Expand Down Expand Up @@ -146,6 +148,7 @@
(pgloader.sources:copy-from source
:state-before state-before
:state-after state-after
:state-indexes state-idx
:truncate truncate
:drop-indexes drop-indexes
:disable-triggers disable-triggers))
Expand All @@ -156,7 +159,8 @@
(when summary
(report-full-summary "Total import time" *state*
:before state-before
:finally state-after))))))
:finally state-after
:parallel state-idx))))))

(defrule load-copy-file load-copy-file-command
(:lambda (command)
Expand Down
6 changes: 5 additions & 1 deletion src/parsers/command-csv.lisp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,8 @@
(let* ((state-before (pgloader.utils:make-pgstate))
(summary (null *state*))
(*state* (or *state* (pgloader.utils:make-pgstate)))
(state-idx ,(when (getf options :drop-indexes)
`(pgloader.utils:make-pgstate)))
(state-after ,(when (or after (getf options :drop-indexes))
`(pgloader.utils:make-pgstate)))
,@(pgsql-connection-bindings pg-db-conn gucs)
Expand Down Expand Up @@ -472,6 +474,7 @@
(pgloader.sources:copy-from source
:state-before state-before
:state-after state-after
:state-indexes state-idx
:truncate truncate
:drop-indexes drop-indexes
:disable-triggers disable-triggers))
Expand All @@ -482,7 +485,8 @@
(when summary
(report-full-summary "Total import time" *state*
:before state-before
:finally state-after))))))
:finally state-after
:parallel state-idx))))))

(defrule load-csv-file load-csv-file-command
(:lambda (command)
Expand Down
10 changes: 7 additions & 3 deletions src/parsers/command-fixed.lisp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,10 @@
(let* ((state-before (pgloader.utils:make-pgstate))
(summary (null *state*))
(*state* (or *state* (pgloader.utils:make-pgstate)))
(state-after ,(when (or after (getf options :drop-indexes))
`(pgloader.utils:make-pgstate)))
(state-idx ,(when (getf options :drop-indexes)
`(pgloader.utils:make-pgstate)))
(state-after ,(when (or after (getf options :drop-indexes))
`(pgloader.utils:make-pgstate)))
,@(pgsql-connection-bindings pg-db-conn gucs)
,@(batch-control-bindings options)
(source-db (with-stats-collection ("fetch" :state state-before)
Expand All @@ -152,6 +154,7 @@
(pgloader.sources:copy-from source
:state-before state-before
:state-after state-after
:state-indexes state-idx
:truncate truncate
:drop-indexes drop-indexes
:disable-triggers disable-triggers))
Expand All @@ -162,7 +165,8 @@
(when summary
(report-full-summary "Total import time" *state*
:before state-before
:finally state-after))))))
:finally state-after
:parallel state-idx))))))

(defrule load-fixed-cols-file load-fixed-cols-file-command
(:lambda (command)
Expand Down
4 changes: 3 additions & 1 deletion src/pgsql/queries.lisp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,13 @@
(defun list-indexes (table-name)
"List all indexes for TABLE-NAME in SCHEMA. A PostgreSQL connection must
be already established when calling that function."
(loop :for (index-name table-name table-oid primary sql conname condef)
(loop :for (index-name table-name table-oid primary unique sql conname condef)
:in (pomo:query (format nil "
select i.relname,
indrelid::regclass,
indrelid,
indisprimary,
indisunique,
pg_get_indexdef(indexrelid),
c.conname,
pg_get_constraintdef(c.oid)
Expand All @@ -214,6 +215,7 @@ select i.relname,
:table-name table-name
:table-oid table-oid
:primary primary
:unique unique
:columns nil
:sql sql
:conname conname
Expand Down
29 changes: 17 additions & 12 deletions src/pgsql/schema.lisp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,20 +293,24 @@

(cols (mapcar #'apply-identifier-case (pgsql-index-columns index))))
(cond
((pgsql-index-condef index)
(format nil "ALTER TABLE ~a ADD ~a;"
table-name (pgsql-index-condef index)))

((pgsql-index-primary index)
((or (pgsql-index-primary index)
(and (pgsql-index-condef index) (pgsql-index-unique index)))
(values
;; ensure good concurrency here, don't take the ACCESS EXCLUSIVE
;; LOCK on the table before we have the index done already
(or (pgsql-index-sql index)
(format nil "CREATE UNIQUE INDEX ~a ON ~a (~{~a~^, ~});"
index-name table-name cols))
(format nil
"ALTER TABLE ~a ADD PRIMARY KEY USING INDEX ~a;"
table-name (pgsql-index-name index))))
"ALTER TABLE ~a ADD ~a USING INDEX ~a;"
table-name
(cond ((pgsql-index-primary index) "PRIMARY KEY")
((pgsql-index-unique index) "UNIQUE"))
(pgsql-index-name index))))

((pgsql-index-condef index)
(format nil "ALTER TABLE ~a ADD ~a;"
table-name (pgsql-index-condef index)))

(t
(or (pgsql-index-sql index)
Expand Down Expand Up @@ -413,27 +417,28 @@
;; and return the indexes list
indexes)))

(defun create-indexes-again (target indexes state &key drop-indexes)
(defun create-indexes-again (target indexes state state-parallel
&key drop-indexes)
"Create the indexes that we dropped previously."
(when (and indexes drop-indexes)
(let* ((idx-kernel (make-kernel (length indexes)))
(idx-channel (let ((lp:*kernel* idx-kernel))
(lp:make-channel))))
(let ((pkeys
(create-indexes-in-kernel target indexes idx-kernel idx-channel
:state state)))
:state state-parallel)))

(with-stats-collection ("Index Build Completion" :state *state*)
(with-stats-collection ("Index Build Completion" :state state)
(loop :for idx :in indexes :do (lp:receive-result idx-channel)))

;; turn unique indexes into pkeys now
(with-pgsql-connection (target)
(with-stats-collection ("Primary Keys" :state state)
(with-stats-collection ("Constraints" :state state)
(loop :for sql :in pkeys
:when sql
:do (progn
(log-message :notice "~a" sql)
(pgsql-execute-with-timing "Primary Keys" sql state)))))))))
(pgsql-execute-with-timing "Constraints" sql state)))))))))

;;;
;;; Sequences
Expand Down
3 changes: 2 additions & 1 deletion src/sources/copy.lisp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@
&key
state-before
state-after
state-indexes
truncate
disable-triggers
drop-indexes)
Expand Down Expand Up @@ -160,6 +161,6 @@
finally (lp:end-kernel))))

;; re-create the indexes
(create-indexes-again (target-db copy) indexes state-after
(create-indexes-again (target-db copy) indexes state-after state-indexes
:drop-indexes drop-indexes)))

3 changes: 2 additions & 1 deletion src/sources/csv/csv.lisp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@
&key
state-before
state-after
state-indexes
truncate
disable-triggers
drop-indexes)
Expand Down Expand Up @@ -216,5 +217,5 @@
finally (lp:end-kernel))))

;; re-create the indexes
(create-indexes-again (target-db csv) indexes state-after
(create-indexes-again (target-db csv) indexes state-after state-indexes
:drop-indexes drop-indexes)))
3 changes: 2 additions & 1 deletion src/sources/fixed.lisp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
&key
state-before
state-after
state-indexes
truncate
disable-triggers
drop-indexes)
Expand Down Expand Up @@ -147,6 +148,6 @@
finally (lp:end-kernel))))

;; re-create the indexes
(create-indexes-again (target-db fixed) indexes state-after
(create-indexes-again (target-db fixed) indexes state-after state-indexes
:drop-indexes drop-indexes)))

4 changes: 2 additions & 2 deletions test/copy.load
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ LOAD COPY
)
INTO postgresql:///pgloader?track_full

WITH truncate
WITH truncate, drop indexes

SET client_encoding to 'latin1',
work_mem to '14MB',
Expand All @@ -15,7 +15,7 @@ LOAD COPY
BEFORE LOAD DO
$$ drop table if exists track_full; $$,
$$ create table track_full (
trackid bigserial,
trackid bigserial primary key,
track text,
album text,
media text,
Expand Down
2 changes: 1 addition & 1 deletion test/partial.load
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ LOAD CSV
BEFORE LOAD DO
$$ drop table if exists partial; $$,
$$ create table partial (
a integer primary key,
a integer unique,
b text,
c text,
d text,
Expand Down

0 comments on commit 54e2977

Please sign in to comment.