Merge pull request #311 from seattleflu/barcode-minting

Add `mint_identifiers` function to improve performance

Author: davereinhart

lib/id3c/db/__init__.py

@@ -4,6 +4,7 @@
 import logging
 import secrets
 import statistics
+import json
 from datetime import datetime
 from psycopg2 import IntegrityError
 from psycopg2.errors import ExclusionViolation
@@ -41,7 +42,6 @@ def mint_identifiers(session: DatabaseSession, name: str, n: int) -> Any:
     `permission denied` error.
     """
     minted: List[Any] = []
-    failures: Dict[int, int] = {}
 
     # Lookup identifier set by name
     identifier_set = session.fetch_row("""
@@ -54,39 +54,24 @@ def mint_identifiers(session: DatabaseSession, name: str, n: int) -> Any:
         LOG.error(f"Identifier set «{name}» does not exist")
         raise IdentifierSetNotFoundError(name)
 
-    started = datetime.now()
+    minted = session.fetch_all("""
+        select uuid, barcode, identifier_set_id, generated
+        from mint_identifiers(%s, %s)
+        """, (identifier_set.id, n))
 
-    while len(minted) < n:
-        m = len(minted) + 1
+    LOG.debug(f"finished minting ")
 
-        LOG.debug(f"Minting identifier {m}/{n}")
+    # capture and log notice from postgres function that contains minting performance stats
+    for notice in session.connection.notices:
+        if 'id3c_minting_performance::' in notice:
+            minting_stats = json.loads(notice.split('::')[-1])
 
-        try:
-            with session.savepoint(f"identifier {m}"):
-                new_identifier = session.fetch_row("""
-                    insert into warehouse.identifier (identifier_set_id, generated)
-                        values (%s, now())
-                        returning uuid, barcode, generated
-                    """,
-                    (identifier_set.id,))
+            duration = minting_stats['exec_time'] / 1000
+            per_second = n / duration
+            per_identifier = duration / n
 
-                minted.append(new_identifier)
-
-        except ExclusionViolation:
-            LOG.debug("Barcode excluded. Retrying.")
-            failures.setdefault(m, 0)
-            failures[m] += 1
-
-    duration = datetime.now() - started
-    per_second = n / duration.total_seconds()
-    per_identifier = duration.total_seconds() / n
-
-    failure_counts = list(failures.values())
-
-    LOG.info(f"Minted {n} identifiers in {n + sum(failure_counts)} tries ({sum(failure_counts)} retries) over {duration} ({per_identifier:.2f} s/identifier = {per_second:.2f} identifiers/s)")
-
-    if failure_counts:
-        LOG.info(f"Failure distribution: max={max(failure_counts)} mode={mode(failure_counts)} median={median(failure_counts)}")
+            LOG.info(f"Minted {minting_stats['count']} identifiers in {minting_stats['count'] + minting_stats['failures']} tries ({minting_stats['failures']} retries) over {duration:.2f} seconds ({per_identifier:.2f} s/identifier = {per_second:.2f} identifiers/s)")
+            LOG.info(f"Failure distribution: max={minting_stats['max']} mode={minting_stats['mode']} median={minting_stats['median']}")
 
     return minted
 

schema/deploy/functions/mint_identifiers.sql

@@ -0,0 +1,59 @@
+-- Deploy seattleflu/schema:functions/mint_identifiers to pg
+
+begin;
+
+create or replace function public.mint_identifiers(set_id integer, number_to_mint integer)
+    returns setof warehouse.identifier
+    language plpgsql as $$
+
+        declare
+            counter integer := 0;
+            r record;
+            failure_count integer := 0;
+            failure_count_arr integer[];
+            error_detail text;
+
+            start_ts timestamptz;
+            exec_time numeric;
+
+            stats jsonb;
+        begin
+            start_ts := clock_timestamp();
+
+            while counter < number_to_mint loop
+                begin
+                    insert into warehouse.identifier (identifier_set_id) values (set_id)
+                    returning uuid, barcode, identifier_set_id, generated into r;
+
+                    raise notice 'Successfully minted identifier: %; barcode: %', r.uuid, r.barcode;
+                    counter := counter + 1;
+                    failure_count_arr[counter] = failure_count;
+                    failure_count := 0;
+                    return next r;
+
+                exception
+                    when others then
+                        GET STACKED DIAGNOSTICS error_detail =  PG_EXCEPTION_DETAIL;
+                        raise notice '%; %', SQLERRM, error_detail;
+                        failure_count := failure_count + 1;
+                end;
+
+            end loop;
+
+            exec_time := 1000 * (extract(epoch FROM clock_timestamp() - start_ts));
+
+            stats := jsonb_build_object('count',counter, 'exec_time', exec_time) ||
+                     (select jsonb_agg(t) -> 0 from (select sum(x) as failures, max(x), mode() within group (order by x), percentile_disc(0.5) within group (order by x) as median from unnest(failure_count_arr) as x) t) as stats;
+
+            raise notice 'id3c_minting_performance::%', stats;
+        end;
+
+
+    $$
+    volatile
+    parallel unsafe;
+
+comment on function public.mint_identifiers(integer, integer) is
+    'Generates and inserts the specified number of valid identifiers for a given identifier set.';
+
+commit;

schema/deploy/roles/identifier-minter/grants.sql

@@ -21,4 +21,12 @@ grant select, insert
    on warehouse.identifier
    to "identifier-minter";
 
+revoke execute
+    on function public.mint_identifiers(integer, integer)
+from public;
+
+grant execute
+    on function public.mint_identifiers(integer, integer)
+    to "identifier-minter";
+
 commit;

schema/deploy/roles/identifier-minter/[email protected]

@@ -0,0 +1,24 @@
+-- Deploy seattleflu/schema:roles/identifier-minter/grants to pg
+-- requires: warehouse/identifier
+-- requires: roles/identifier-minter/create
+
+begin;
+
+-- This change is designed to be sqitch rework-able to make it easier to update
+-- the grants for this role.
+
+grant connect on database :"DBNAME" to "identifier-minter";
+
+grant usage
+   on schema warehouse
+   to "identifier-minter";
+
+grant select
+   on warehouse.identifier_set
+   to "identifier-minter";
+
+grant select, insert
+   on warehouse.identifier
+   to "identifier-minter";
+
+commit;

schema/revert/functions/mint_identifiers.sql

@@ -0,0 +1,7 @@
+-- Revert seattleflu/schema:functions/mint_identifiers from pg
+
+begin;
+
+drop function if exists public.mint_identifiers(integer, integer);
+
+commit;

schema/revert/roles/identifier-minter/grants.sql

@@ -1,19 +1,28 @@
--- Revert seattleflu/schema:roles/identifier-minter/grants from pg
+-- Deploy seattleflu/schema:roles/identifier-minter/grants to pg
+-- requires: warehouse/identifier
+-- requires: roles/identifier-minter/create
 
 begin;
 
-revoke select, insert
-  on warehouse.identifier
-  from "identifier-minter";
+-- This change is designed to be sqitch rework-able to make it easier to update
+-- the grants for this role.
 
-revoke select
-  on warehouse.identifier_set
-  from "identifier-minter";
+grant connect on database :"DBNAME" to "identifier-minter";
 
-revoke usage
-    on schema warehouse
-  from "identifier-minter";
+grant usage
+   on schema warehouse
+   to "identifier-minter";
 
-revoke connect on database :"DBNAME" from "identifier-minter";
+grant select
+   on warehouse.identifier_set
+   to "identifier-minter";
+
+grant select, insert
+   on warehouse.identifier
+   to "identifier-minter";
+
+revoke execute
+    on function public.mint_identifiers(integer, integer)
+from "identifier-minter";
 
 commit;

schema/revert/roles/identifier-minter/[email protected]

@@ -0,0 +1,19 @@
+-- Revert seattleflu/schema:roles/identifier-minter/grants from pg
+
+begin;
+
+revoke select, insert
+  on warehouse.identifier
+  from "identifier-minter";
+
+revoke select
+  on warehouse.identifier_set
+  from "identifier-minter";
+
+revoke usage
+    on schema warehouse
+  from "identifier-minter";
+
+revoke connect on database :"DBNAME" from "identifier-minter";
+
+commit;

schema/sqitch.plan

@@ -237,3 +237,9 @@ warehouse/identifier/indexes/identifier_set_id_btree 2021-07-06T22:33:41Z Dave R
 roles/sample-editor/create 2021-08-13T15:01:31Z Dave Reinhart <[email protected]> # Add sample editor role
 roles/sample-editor/grants 2021-08-13T15:03:19Z Dave Reinhart <[email protected]> # Grant permissions to sample-editor role
 @2021-08-13 2021-08-13T15:15:55Z Dave Reinhart <[email protected]> # Schema as of 13 Aug 2021
+
+functions/mint_identifiers 2022-07-15T22:13:52Z Dave Reinhart <[email protected]> # Adding mint_identifiers function to batch mint identifiers for a given set.
+@2022-07-25 2022-07-25T21:27:47Z Dave Reinhart <[email protected]> # Schemas as of 25 July 2022
+
+roles/identifier-minter/grants [roles/identifier-minter/grants@2022-07-25] 2022-07-28T19:05:20Z Dave Reinhart <[email protected]> # Add execute permissions on mint_identifiers function to identiifer-minter
+@2022-07-28 2022-07-28T19:20:25Z Dave Reinhart <[email protected]> # Schema as of 28 July 2022

schema/verify/functions/mint_identifiers.sql

@@ -0,0 +1,22 @@
+-- Verify seattleflu/schema:functions/mint_identifiers on pg
+
+begin;
+
+do $$
+    declare
+        test record;
+    begin
+        create temp table if not exists tests
+            as select uuid, barcode, identifier_set_id, generated
+            from public.mint_identifiers(6, 3);
+
+        assert (select count(*) from tests) = 3;
+
+        for test in table tests loop
+            assert test.identifier_set_id = 6;
+        end loop;
+
+    end
+$$;
+
+rollback;

schema/verify/roles/identifier-minter/grants.sql

@@ -6,9 +6,11 @@ select 1/pg_catalog.has_database_privilege('identifier-minter', :'DBNAME', 'conn
 select 1/pg_catalog.has_schema_privilege('identifier-minter', 'warehouse', 'usage')::int;
 select 1/pg_catalog.has_table_privilege('identifier-minter', 'warehouse.identifier', 'select,insert')::int;
 select 1/pg_catalog.has_table_privilege('identifier-minter', 'warehouse.identifier_set', 'select')::int;
+select 1/pg_catalog.has_function_privilege('identifier-minter', 'public.mint_identifiers(integer,integer)', 'execute')::int;
 
 select 1/(not pg_catalog.has_schema_privilege('identifier-minter', 'receiving', 'usage'))::int;
 select 1/(not pg_catalog.has_table_privilege('identifier-minter', 'warehouse.identifier', 'update,delete'))::int;
 select 1/(not pg_catalog.has_table_privilege('identifier-minter', 'warehouse.identifier_set', 'insert,update,delete'))::int;
+select 1/(not pg_catalog.has_function_privilege('public', 'public.mint_identifiers(integer,integer)', 'execute'))::int;
 
 rollback;

schema/verify/roles/identifier-minter/[email protected]

@@ -0,0 +1,14 @@
+-- Verify seattleflu/schema:roles/identifier-minter/grants on pg
+
+begin;
+
+select 1/pg_catalog.has_database_privilege('identifier-minter', :'DBNAME', 'connect')::int;
+select 1/pg_catalog.has_schema_privilege('identifier-minter', 'warehouse', 'usage')::int;
+select 1/pg_catalog.has_table_privilege('identifier-minter', 'warehouse.identifier', 'select,insert')::int;
+select 1/pg_catalog.has_table_privilege('identifier-minter', 'warehouse.identifier_set', 'select')::int;
+
+select 1/(not pg_catalog.has_schema_privilege('identifier-minter', 'receiving', 'usage'))::int;
+select 1/(not pg_catalog.has_table_privilege('identifier-minter', 'warehouse.identifier', 'update,delete'))::int;
+select 1/(not pg_catalog.has_table_privilege('identifier-minter', 'warehouse.identifier_set', 'insert,update,delete'))::int;
+
+rollback;