mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git

Valery Kharseko
2 days ago 8503e132a947cfd7f756a42ae0f8cef926426220
opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java
@@ -635,6 +635,13 @@
   private final AtomicBoolean ddlLockBoundNotSetWarned = new AtomicBoolean();
   private final AtomicLong ddlLockBoundLeftBehindWarned = new AtomicLong();
   private static final long DDL_LOCK_BOUND_WARNING_INTERVAL_MS = 10000;
   // And a third way, which is no failure of anything: an engine this backend knows no lock setting
   // for is left unbounded deliberately, and says so once - see reportTheEngineIsNotKnown(). Not
   // private, so that a case can read what a storage has already said without reading a log.
   final AtomicBoolean ddlLockBoundEngineUnknownWarned = new AtomicBoolean();
   // What was actually said, set beside the flag above and read the same way: the flag alone tells a
   // case that something was logged, not what it named.
   volatile String ddlLockBoundEngineUnknownSaid;
   /**
    * The socket read timeout of one connection, and the statements running on it. This second
@@ -1301,6 +1308,12 @@
    * {@code dsconfig create-backend-index} is answered by {@code alter system set ddl_lock_timeout},
    * not by this.
    * <p>
    * An engine behind a driver this backend does not know is left alone as well, and is told about
    * rather than left silent - see {@code reportTheEngineIsNotKnown()}. {@link #dialectOf} reads the
    * engine off the class name of the driver, so a mariadb, percona or aurora driver against a live
    * mysql is one of these: the bound stays off there, and the line saying so is what an operator has
    * to go on.
    * <p>
    * What it costs is the round trips of the statements around each DDL - three on mysql and sql
    * server (reading the value back, setting the bound, giving the value back), two on postgres (the
    * savepoint a failed setting is taken back to, and the setting), none on oracle - and only on the
@@ -2127,12 +2140,15 @@
    * this reachable from a test with no database behind it.
    * <p>
    * Nothing of ours is set where it could not be taken off again, and a failure of the readback is
    * never the failure of the DDL: this runs on a pooled connection, so a setting left behind reaches
    * every statement of whoever borrows it next - on sql server that is every lock wait of theirs,
    * row locks included, and {@link #isConflict} classifies error 1222 as no replayable conflict. A
    * session this backend could not take its bound off again is kept out of the pool for that reason
    * ({@link CachedConnection#keepOutOfThePool}), since the validation of the next borrow is
    * {@code isValid()} - a liveness check a connection carrying a stale setting passes.
    * never the failure of the DDL: this runs mostly on a pooled connection, so a setting left behind
    * reaches every statement of whoever borrows it next - on sql server that is every lock wait of
    * theirs, row locks included, and {@link #isConflict} classifies error 1222 as no replayable
    * conflict. A session this backend could not take its bound off again is kept out of the pool for
    * that reason ({@link CachedConnection#keepOutOfThePool}), since the validation of the next borrow
    * is {@code isValid()} - a liveness check a connection carrying a stale setting passes. The one
    * connection here that is not the pool's is the catalog's own, which {@code createCatalogTable()}
    * creates its table on: there a setting left behind reaches the rest of that write and goes with
    * the connection, which is closed with it.
    * <p>
    * The DDL runs whatever any of that did, and it runs under the same rewrite either way: a setting
    * can reach the server and fail only as the statement carrying it is closed, which no driver tells
@@ -2141,10 +2157,21 @@
    */
   <T> T withDdlLockBound(Connection con, Dialect dialect, Execution<T> action) throws SQLException {
      final int seconds=ddlLockBoundSeconds();
      // Asked first with nothing displaced yet, which is what tells an engine this bound is never put
      // on - oracle, and one none of these settings fit - from an engine it is put on. What the session
      // actually carries is read below, and can take the bound off again all by itself.
      if (dialect==null || seconds<=0 || dialect.ddlLockBoundSql(seconds, null)==null) {
      if (seconds<=0) { // the wait is left exactly as unbounded as it was, and nobody asked otherwise
         return action.run();
      }
      if (dialect==null) {
         // The one branch where a bound was asked for and none is put on, which is why it is the one
         // that says so: an engine none of these settings fit is fed none of them - untested SQL is no
         // thing to send a database on the path a backend opens by - and the silence around that is
         // what an operator has no way of finding out.
         reportTheEngineIsNotKnown(con);
         return action.run();
      }
      // Asked with nothing displaced yet, which is what tells an engine this bound is never put on -
      // oracle - from an engine it is put on. What the session actually carries is read below, and can
      // take the bound off again all by itself.
      if (dialect.ddlLockBoundSql(seconds, null)==null) {
         return action.run();
      }
      final String query=dialect.ddlLockBoundQuery();
@@ -2338,11 +2365,50 @@
   }
   /**
    * Said once per storage, where the engine behind a connection is not one this backend knows a lock
    * setting for. Leaving the bound off such an engine is the conservative reading and stays - untested
    * SQL is no thing to send a database on the path a backend opens by - but a deployment that asked
    * for the bound has no way of finding out that it got none, which is the silence this ends. It is
    * the argument the strict parsing of {@value #DDL_LOCK_TIMEOUT_PROPERTY} is made with, one property
    * later.
    * <p>
    * {@link #dialectOf} reads the engine off the class name of the driver, so this is not the engine
    * of an exotic database alone: a mariadb, percona or aurora driver against a live mysql answers
    * null here, and that is a session whose {@code lock_wait_timeout} is a year - the very wait this
    * bound exists to end. {@link CachedConnection} says the same of a url it knows no connect bound
    * for and cannot say it for this one: it keys on the url, which such a driver takes as a mysql one.
    * <p>
    * The driver is named rather than the url, since the driver is what this reads and what a
    * deployment would change - and a url carries the password of the account this backend works as.
    * <p>
    * The create table this report is issued beside ({@code createCatalogTable()}, and the one
    * {@code openTree()} issues for a tree of its own) is not itself a wait such an engine leaves
    * unbounded: {@code getTableDialect()} answers an unrecognised engine with postgres' column
    * types, which a mysql-family server rejects as a syntax error before it ever queues for a lock.
    * What this warns of - the wait an operator would otherwise meet with no word on why - is the
    * create index of an open whose table is already there, and the drop table of a clear.
    */
   private void reportTheEngineIsNotKnown(Connection con) {
      if (ddlLockBoundEngineUnknownWarned.compareAndSet(false, true)) {
         final String said=String.format("jdbc: the wait of a DDL for a lock is left unbounded on this"
            + " database: %s is not a driver this backend knows a lock setting of an engine for, so %s"
            + " bounds nothing here and a DDL - the create index of an open whose table is already there,"
            + " the drop table of a clear - waits for a lock another session holds for as long as this"
            + " engine lets it", driverNameOf(con), DDL_LOCK_TIMEOUT_PROPERTY);
         ddlLockBoundEngineUnknownSaid=said; // read back by a case the way it reads the flag beside it
         logger.warn(LocalizableMessage.raw(said));
      }
   }
   /**
    * Gives the session back the value it carried. Best effort, and never the outcome of the DDL: this
    * runs from a {@code finally} while the caller may be being unwound, where a throw would replace
    * the failure that brought it there (JLS 14.20.2) - the very one saying what went wrong.
    * <p>
    * A connection this failed on does not go back into the pool. Leaving it to the next borrow to
    * A connection this failed on is handed on to nobody. A pooled one is closed rather than given
    * back, and the catalog's own - the one connection reaching this that was never in the pool -
    * carries the setting for the rest of the write that opened it, the enrolment of the tree among
    * that, and is closed with that write. Leaving it to the next borrow to
    * notice does not work: that validation is {@code con.isValid()}, a liveness check which a
    * connection whose reset failed for a transient reason passes while still carrying our bound, and
    * on sql server it would then cut every lock wait of that borrower at it - row locks included,
@@ -2371,8 +2437,10 @@
         if (now-last >= DDL_LOCK_BOUND_WARNING_INTERVAL_MS && ddlLockBoundLeftBehindWarned.compareAndSet(last, now)) {
            logger.warn(LocalizableMessage.raw("jdbc: the lock bound of a DDL could not be taken off a connection"
               + " of this %s database, which may have been left carrying \"%s\" instead of the value it had:"
               + " that connection is closed rather than pooled, so no borrow after this one gives up on a lock"
               + " at a bound of %s it never asked for (%s)", dialect, bound, DDL_LOCK_TIMEOUT_PROPERTY,
               + " that connection is not handed on - a pooled one is closed rather than given back, and the"
               + " catalog's own carries it for the rest of the write that opened it and is closed with that"
               + " write - so no later write gives up on a lock at a bound of %s it never asked for (%s)",
               dialect, bound, DDL_LOCK_TIMEOUT_PROPERTY,
               stackTraceToSingleLineString(e)));
         }
      }
@@ -4756,17 +4824,43 @@
       * An account that may write its rows but not create a table is a configuration this can meet,
       * so the failure says which table it was and why the backend wanted it, rather than reaching
       * the operator as a bare SQL error inside ERR_OPEN_ENV_FAIL.
       * <p>
       * It waits for its lock under {@link JDBCStorage#DDL_LOCK_TIMEOUT_PROPERTY} like every other DDL
       * of this backend, and a lock it gives up on names that property: this statement is issued on the
       * catalog's own connection rather than through {@code commitStatement()}, which is the funnel
       * that bounds the rest, so the bound is put on here. The catch below tells a lock from the
       * privilege line, and reads the failure the way {@link JDBCStorage#lockNotAvailable} does rather
       * than by the class {@code gaveUpOnTheLock()} renames to: that rename reaches only a wait this
       * backend's own bound ended, and a lock is a lock on every road the create runs bare on as well -
       * the property at 0, a session already giving up sooner than ours, oracle left to its
       * {@code ddl_lock_timeout}, a wait that ran past the slack - where it arrives as the engine's own
       * 55P03 / 1205 / ORA-00054 / 1222. A create the bound of its own class cut short
       * ({@code bulk.timeout}, which {@code timedOut()} has already named) is neither, and carries that
       * line. What is left is the privilege the account is missing. An engine this backend does not
       * know has no number a lock could be told by ({@code isLockTimeout()} answers false on a null
       * dialect), and gets the privilege line there as it did.
       */
      void createCatalogTable(TreeName catalog) {
         final String tableName=getTableName(catalog);
         Dialect dialect=null; // read inside the try, and asked again by the catch, which tells a lock by the engine's own number
         try {
            final Connection catalogCon=catalogSession.connection();
            try (final PreparedStatement statement=catalogCon.prepareStatement("create table "+tableName+" ("+getTableDialect()+")")) {
               // bulk like every other create table of this backend (#882): it is DDL nobody waits on,
               // and the class of a client operation is not what a statement of this kind can be given
               execute(statement, StatementBound.BULK);
            }
            catalogCon.commit();
            dialect=dialectOf(catalogCon);
            // Under the same bound as every other DDL of this backend, although this one reaches no
            // commitStatement(): it is a create table of an open like the ones openTree() issues, and
            // it queues for the same kind of lock - another process creating this very table inside a
            // transaction it has not committed is a wait three engines out of four never end. The
            // commit is inside the bound because on postgres it is that commit which ends the
            // transaction a "set local" belongs to.
            withDdlLockBound(catalogCon, dialect, () -> {
               try (final PreparedStatement statement=catalogCon.prepareStatement("create table "+tableName+" ("+getTableDialect()+")")) {
                  // bulk like every other create table of this backend (#882): it is DDL nobody waits on,
                  // and the class of a client operation is not what a statement of this kind can be given
                  execute(statement, StatementBound.BULK);
               }
               catalogCon.commit();
               return null;
            });
         } catch (SQLException | RuntimeException e) {
            // the unchecked one as well, for the reason enrolInCatalog() takes it: what the statement
            // left behind has to be rolled back whatever class the failure arrived in, this connection
@@ -4792,9 +4886,21 @@
                  tableName, stackTraceToSingleLineString(e)));
               return;
            }
            // A lock another session holds is not a privilege the account lacks, whichever bound ended
            // the wait for it: asked of the engine's own number on every road, since gaveUpOnTheLock()
            // renames only a wait this backend's own bound ended, and the create runs bare wherever
            // withDdlLockBound() puts no bound on. Asked without the release, as lockNotAvailable()
            // always is: the lookup above has just been suppressed into this failure, and what it met
            // on its way says nothing about what the create did. A create the bound of its own class
            // cut short is neither, and timedOut() has already named that property on it.
            final String why=lockNotAvailable(e, dialect)
               ? "the create waited for a lock another session holds and gave up: "+e.getMessage()
               : (e instanceof SQLTimeoutException)
                  ? "the create was ended by the bound of its own class: "+e.getMessage()
                  : "a read-write open of a JDBC backend needs the privilege to create it, and a clear of"
                     +" one names nothing without it";
            throw new StorageRuntimeException("jdbc: backend "+config.getBackendId()+" could not create table "
               +tableName+", which holds the catalog naming the trees it owns: a read-write open of a JDBC"
               +" backend needs the privilege to create it, and a clear of one names nothing without it", e);
               +tableName+", which holds the catalog naming the trees it owns: "+why, e);
         }
      }