From 54e29773d7fe5482eb3693a24a465c90f9658204 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Sat, 18 Jul 2015 23:06:15 +0200 Subject: [PATCH] Fix index creation reporting, see #251. 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. --- src/parsers/command-copy.lisp | 6 +++++- src/parsers/command-csv.lisp | 6 +++++- src/parsers/command-fixed.lisp | 10 +++++++--- src/pgsql/queries.lisp | 4 +++- src/pgsql/schema.lisp | 29 +++++++++++++++++------------ src/sources/copy.lisp | 3 ++- src/sources/csv/csv.lisp | 3 ++- src/sources/fixed.lisp | 3 ++- test/copy.load | 4 ++-- test/partial.load | 2 +- 10 files changed, 46 insertions(+), 24 deletions(-) diff --git a/src/parsers/command-copy.lisp b/src/parsers/command-copy.lisp index b6eab68d..88a55f09 100644 --- a/src/parsers/command-copy.lisp +++ b/src/parsers/command-copy.lisp @@ -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) @@ -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)) @@ -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) diff --git a/src/parsers/command-csv.lisp b/src/parsers/command-csv.lisp index 9247074e..9c076379 100644 --- a/src/parsers/command-csv.lisp +++ b/src/parsers/command-csv.lisp @@ -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) @@ -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)) @@ -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) diff --git a/src/parsers/command-fixed.lisp b/src/parsers/command-fixed.lisp index 9c1f3168..f82276fb 100644 --- a/src/parsers/command-fixed.lisp +++ b/src/parsers/command-fixed.lisp @@ -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) @@ -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)) @@ -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) diff --git a/src/pgsql/queries.lisp b/src/pgsql/queries.lisp index e4b933fb..593a36a6 100644 --- a/src/pgsql/queries.lisp +++ b/src/pgsql/queries.lisp @@ -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) @@ -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 diff --git a/src/pgsql/schema.lisp b/src/pgsql/schema.lisp index 95e546e8..137e4fe8 100644 --- a/src/pgsql/schema.lisp +++ b/src/pgsql/schema.lisp @@ -293,11 +293,8 @@ (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 @@ -305,8 +302,15 @@ (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) @@ -413,7 +417,8 @@ ;; 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))) @@ -421,19 +426,19 @@ (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 diff --git a/src/sources/copy.lisp b/src/sources/copy.lisp index 30db9938..1807f1e2 100644 --- a/src/sources/copy.lisp +++ b/src/sources/copy.lisp @@ -120,6 +120,7 @@ &key state-before state-after + state-indexes truncate disable-triggers drop-indexes) @@ -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))) diff --git a/src/sources/csv/csv.lisp b/src/sources/csv/csv.lisp index f607b0cc..9006efd8 100644 --- a/src/sources/csv/csv.lisp +++ b/src/sources/csv/csv.lisp @@ -177,6 +177,7 @@ &key state-before state-after + state-indexes truncate disable-triggers drop-indexes) @@ -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))) diff --git a/src/sources/fixed.lisp b/src/sources/fixed.lisp index 04152a8d..bde1c0f6 100644 --- a/src/sources/fixed.lisp +++ b/src/sources/fixed.lisp @@ -107,6 +107,7 @@ &key state-before state-after + state-indexes truncate disable-triggers drop-indexes) @@ -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))) diff --git a/test/copy.load b/test/copy.load index 94f69b9f..1c81cb7a 100644 --- a/test/copy.load +++ b/test/copy.load @@ -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', @@ -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, diff --git a/test/partial.load b/test/partial.load index 06a1a80b..156d0c87 100644 --- a/test/partial.load +++ b/test/partial.load @@ -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,