Ops fixes (#3616)

* Ops fixes

Correct accounting and reporting of cache unpacking statistics.

Avoid reflecting SQLAlchemy `init_db` errors into conmon.

I realized that even though I'd shortened the traceback, it would still be
displayed by Python on termination with an unhandled exception, and that would
still be relayed through conmon one line at a time. To avoid that, I added an
`except Exception` clause to print a simple message and exit.

In the process, I realized that while I'd written a new Click-based
`pbench-reindex`, the old `/opt/pbench-server/bin/pbench-reindex` script was
still there, and referenced in several config files. I removed these.

.flake8

@@ -3,7 +3,6 @@ doctests = True
 filename =
         *.py
         *server/bin/pbench-index
-        *server/bin/pbench-reindex
 exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build,*web-server/*,
         *agent/bench-scripts/test-bin/fio-histo-log-pctiles.py,
         *agent/bench-scripts/tests/pbench-trafficgen/test-39.trafficgen/*,

lib/pbench/cli/server/report.py

@@ -176,10 +176,11 @@ def report_cache(tree: CacheManager):
     lacks_metrics = 0
     bad_metrics = 0
     unpacked_count = 0
-    unpacked_times = 0
+    unpacked_multiple = 0
     stream_unpack_skipped = 0
     last_ref_errors = 0
     agecomp = Comparator("age", really_big=time.time() * 2.0)
+    unpackcomp = Comparator("unpack")
     sizecomp = Comparator("size")
     compcomp = Comparator("compression")
     speedcomp = Comparator("speed", really_big=GINORMOUS_FP)
@@ -254,7 +255,9 @@ def report_cache(tree: CacheManager):
         else:
             found_metrics = True
             unpacked_count += 1
-            unpacked_times += metrics["count"]
+            if metrics["count"] > 1:
+                unpacked_multiple += 1
+            unpackcomp.add(dsname, metrics["count"])
             speedcomp.add(dsname, metrics["min"], metrics["max"])
             if size:
                 stream_fast = size / metrics["min"] / MEGABYTE_FP
@@ -270,9 +273,14 @@ def report_cache(tree: CacheManager):
         f"{humanize.naturalsize(cached_size)}"
     )
     click.echo(
-        f"  {unpacked_count:,d} datasets have been unpacked a total of "
-        f"{unpacked_times:,d} times"
+        f"  {unpacked_count:,d} datasets have unpack metrics and "
+        f"{unpacked_multiple} have been unpacked more than once."
     )
+    if unpackcomp.max > 1 or verifier.verify:
+        click.echo(
+            f"  The most unpacked dataset has been unpacked {unpackcomp.max} times, "
+            f"{unpackcomp.max_name}"
+        )
     click.echo(
         "  The least recently used cache was referenced "
         f"{humanize.naturaldate(oldest)}, {agecomp.min_name}"
@@ -318,12 +326,9 @@ def report_cache(tree: CacheManager):
             f"{last_ref_errors:,d} are missing reference timestamps, "
             f"{bad_size:,d} have bad size metadata"
         )
-    if lacks_metrics or bad_metrics or verifier.verify:
-        click.echo(
-            f"  {lacks_metrics:,d} datasets are missing unpack metric data, "
-            f"{bad_metrics} have bad unpack metric data"
-        )
-    if lacks_tarpath:
+    if bad_metrics or verifier.verify:
+        click.echo(f"  {bad_metrics:,d} have bad unpack metric data")
+    if lacks_tarpath or verifier.verify:
         click.echo(
             f"  {lacks_tarpath} datasets are missing server.tarball-path metadata"
         )

lib/pbench/cli/server/shell.py

@@ -222,6 +222,7 @@ def main() -> int:
 
     try:
         return run_gunicorn(server_config, logger)
-    except Exception:
-        logger.exception("Unhandled exception running gunicorn")
+    except Exception as exc:
+        logger.exception("Unhandled exception running gunicorn: {!r}", str(exc))
+        print(exc, file=sys.stderr)
         return 1

lib/pbench/server/database/database.py

@@ -11,6 +11,10 @@
 from pbench.server import PbenchServerConfig
 
 
+class DatabaseInitError(Exception):
+    pass
+
+
 class Database:
     # Create declarative base model that our model can inherit from
     Base = declarative_base()
@@ -64,33 +68,38 @@ def init_db(server_config: PbenchServerConfig, logger: Logger):
         # metadata will not have any tables and create_all functionality will do
         # nothing.
 
-        engine = create_engine(db_uri)
-        db_url = urlparse(db_uri)
-        if db_url.scheme == "sqlite":
-            Database.Base.metadata.create_all(bind=engine)
-        else:
-            alembic_cfg = Config()
-            alembic_cfg.set_main_option(
-                "script_location", str(Path(__file__).parent / "alembic")
+        try:
+            engine = create_engine(db_uri)
+            db_url = urlparse(db_uri)
+            if db_url.scheme == "sqlite":
+                Database.Base.metadata.create_all(bind=engine)
+            else:
+                alembic_cfg = Config()
+                alembic_cfg.set_main_option(
+                    "script_location", str(Path(__file__).parent / "alembic")
+                )
+                alembic_cfg.set_main_option("prepend_sys_path", ".")
+                with engine.begin() as connection:
+                    alembic_cfg.attributes["connection"] = connection
+                    command.upgrade(alembic_cfg, "head")
+            Database.db_session = scoped_session(
+                sessionmaker(bind=engine, autocommit=False, autoflush=False)
+            )
+            Database.Base.query = Database.db_session.query_property()
+
+            # Although most of the Pbench Server currently works with the default
+            # SQLAlchemy transaction management, some parts rely on true atomic
+            # transactions and need a better isolation level.
+            # NOTE: In PostgreSQL we might use a slightly looser integrity level
+            # like "REPEATABLE READ", however as this isn't supported in sqlite3
+            # we're using the strictest "SERIALIZABLE" level.
+            Database.maker = sessionmaker(
+                bind=engine.execution_options(isolation_level="SERIALIZABLE")
             )
-            alembic_cfg.set_main_option("prepend_sys_path", ".")
-            with engine.begin() as connection:
-                alembic_cfg.attributes["connection"] = connection
-                command.upgrade(alembic_cfg, "head")
-        Database.db_session = scoped_session(
-            sessionmaker(bind=engine, autocommit=False, autoflush=False)
-        )
-        Database.Base.query = Database.db_session.query_property()
-
-        # Although most of the Pbench Server currently works with the default
-        # SQLAlchemy transaction management, some parts rely on true atomic
-        # transactions and need a better isolation level.
-        # NOTE: In PostgreSQL we might use a slightly looser integrity level
-        # like "REPEATABLE READ", however as this isn't supported in sqlite3
-        # we're using the strictest "SERIALIZABLE" level.
-        Database.maker = sessionmaker(
-            bind=engine.execution_options(isolation_level="SERIALIZABLE")
-        )
+        except Exception as e:
+            if logger:
+                logger.error("Unable to initialize database {}: {!r}", db_uri, str(e))
+            raise DatabaseInitError(str(e))
 
     @staticmethod
     def dump_query(query: Query, logger: Logger, level: int = DEBUG):

pyproject.toml

@@ -44,7 +44,6 @@ include = '''
 | ^/agent/util-scripts/validate-ipaddress$
 | ^/agent/util-scripts/pbench-tool-meister-start$
 | ^/server/bin/pbench-index$
-| ^/server/bin/pbench-reindex$
 | ^/utils/fetch-curr-commit-id$
 '''
 extend-exclude = '''

server/bin/pbench-index

@@ -204,4 +204,10 @@ if __name__ == "__main__":
         # exit status to 1.  All finally handlers would have been executed, no
         # need to report a SIGTERM again.
         status = 1
+    except Exception as e:
+        # Don't let an unhandled exception generate a potentially massive
+        # traceback that's unlikely to have much real value (and which will be
+        # echoed to syslogd by conmon, one line at a time).
+        print(f"Indexing failed with exception {str(e)!r}", file=sys.stderr)
+        status = 1
     sys.exit(status)