[#961] Walk every link of a failure where the verdict decides something (#1...

Valery Kharseko

22 hours ago 240fd3611e01dbb0a4b22ac371e495ed9e53d8aa

[#961] Walk every link of a failure where the verdict decides something (#1004)

 opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java

@@ -89,29 +89,28 @@
    private static final double MAX_SLEEP_ON_RETRY_MS = 1000.0;

    /**
     * Number of links walked by the questions that are not asked to the end of the chains: a guard against a chain
     * long enough to matter, at the cost of what truncation costs each of them. One number for three chains at
     * once - the causes, the next exceptions and the suppressed exceptions are walked together and counted
     * together - so it is set well above the depth a wrapped failure of this backend reaches: mssql-jdbc chains
     * every error of one message it received through {@code setNextException}, and a budget spent on those would
     * never reach the cause the wrapper carries.
     * Number of links walked by the one question of this class whose answer truncation makes less precise rather
     * than wrong: the fallback of {@link #conflictSummary} that names the first {@link SQLException} of a failure,
     * where a link past the budget costs a less precise line in the log and no decision. Every question a decision
     * reads is asked with {@link #EVERY_LINK} instead: a budget does not leave those unanswered, it answers them
     * with the verdict of a failure that carries nothing - which is what issue #961 was.
     * <p>
     * For the fallbacks of {@link #conflictSummary} truncation leaves a question unanswered and nothing more: the
     * line reporting the replay names a less precise link, and no decision moves. For
     * {@link #isConnectionFailure} it does weaken the verdict, which is the test {@link #EVERY_LINK} exists to
     * apply: a class 08 link past this many links leaves {@code dropped} false in {@link #write}, so
     * {@link #distrustPool} is not called and the pool keeps handing out - unvalidated - the connections it had
     * established before the same restart or failover. That is what this walk did before #903 and it is left as it
     * is here rather than widened along with the two below, since nothing about the grant of #903 depends on it;
     * the budget is pinned from both sides by {@code testTheWalkOfAFailureStopsAtItsBudget}, which is where
     * widening it would have to start.
     * One number for three chains at once - the causes, the next exceptions and the suppressed exceptions are walked
     * together and counted together - so it is set well above the depth a wrapped failure of this backend reaches:
     * mssql-jdbc chains every error of one message it received through {@code setNextException}, and a budget spent
     * on those would never reach the cause the wrapper carries.
     */
    private static final int MAX_CHAIN_LINKS = 64;

    /**
     * The budget of the two walks whose verdict would weaken rather than go unnoticed under truncation:
     * {@link #failureScope} and {@link #conflictVerdict}. See the comments above them; the {@code seen} set of
     * the walk terminates it either way.
     * The budget of every walk of this class but one: {@link #failureScope}, {@link #isConnectionFailure},
     * {@link #conflictVerdict}, {@link #lockNotAvailable} and the lookup of {@link #conflictSummary} that mirrors
     * {@link #isConnectionFailure} all read to the end of the chains - see the comments above them. A budget does
     * not leave a question a decision reads unanswered, it answers it with the verdict of a failure that carries
     * nothing, which is what issue #961 was. The one exception is what {@link #MAX_CHAIN_LINKS} is left for, where
     * truncation costs the precision of a line of the log and no decision. The {@code seen} set terminates the walk
     * whatever budget it is given, so this one costs a walk of the links the driver has already allocated and
     * nothing else.
     */
    private static final int EVERY_LINK = Integer.MAX_VALUE;

@@ -3909,9 +3908,16 @@
     * 14.20.3.1) rather than as a cause. The walk starts at the failure this class was handed because it reaches it
     * wrapped in a {@link StorageRuntimeException}, and a caller such as {@code EntryContainer.addEntry} may wrap
     * it once more.
     * <p>
     * Every link of them is walked, rather than {@link #MAX_CHAIN_LINKS} of them, for the reason
     * {@link #failureScope} walks every link: truncation does not leave this question unanswered, it answers it with
     * the {@code false} of a failure that says nothing about the connection. A class 08 link past a budget leaves
     * {@code dropped} false in {@link #write} and in {@link #read}, so {@link #distrustPool} is not called and the
     * pool keeps handing out - unvalidated, for the rest of the alive window - the connections the same restart or
     * failover took, and {@link #replayReason} fails an attempt that was worth replaying (issue #961).
     */
    static boolean isConnectionFailure(Throwable failure) {
        return firstLinkMatching(failure, WITH_THE_RELEASE, JDBCStorage::saysTheConnectionIsGone)!=null;
        return firstLinkMatching(failure, WITH_THE_RELEASE, EVERY_LINK, JDBCStorage::saysTheConnectionIsGone)!=null;
    }

    /**
@@ -3928,13 +3934,11 @@
     * The first {@link SQLException} of the chains of a failure that answers the given question, or null where none
     * does. Every classifier of this class walks the failure this way, so that none of them reads a chain the others
     * act on: what makes a write replayable must also be what the pool is told about and what the replay logs.
     * <p>
     * The number of links it may look at is named by every caller rather than defaulted, so that a walk whose
     * verdict decides something cannot inherit a budget without saying so: {@link #EVERY_LINK} for the questions a
     * decision reads, {@link #MAX_CHAIN_LINKS} for the one that only names a link in a line of the log.
     */
    private static SQLException firstLinkMatching(Throwable failure, boolean withTheRelease,
            Predicate<SQLException> matches) {
        return firstLinkMatching(failure, withTheRelease, MAX_CHAIN_LINKS, matches);
    }

    /** The walk above, with the number of links it is allowed to look at. */
    private static SQLException firstLinkMatching(Throwable failure, boolean withTheRelease, int links,
            Predicate<SQLException> matches) {
        final SQLException[] found=new SQLException[1];
@@ -4263,13 +4267,14 @@
        // that merely resembles the one the decision was taken on
        SQLException named=verdict.link;
        if (named==null) {
            named=firstLinkMatching(failure, WITH_THE_RELEASE, JDBCStorage::saysTheConnectionIsGone);
            named=firstLinkMatching(failure, WITH_THE_RELEASE, EVERY_LINK, JDBCStorage::saysTheConnectionIsGone);
        }
        if (named==null) {
            // without the release, so that the line names the statement that failed rather than the rollback
            // behind it: this is the fallback of a replay decided on isClosed(con) alone, where neither chain
            // carries a verdict, and the walk reaches the suppressed exceptions before the cause
            named=firstLinkMatching(failure, WITHOUT_THE_RELEASE, e -> true);
            // carries a verdict, and the walk reaches the suppressed exceptions before the cause. The one walk
            // left bounded: no decision reads it, so a link past the budget costs the precision of this line
            named=firstLinkMatching(failure, WITHOUT_THE_RELEASE, MAX_CHAIN_LINKS, e -> true);
        }
        return named==null
            ? String.valueOf(failure)

 opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStorageRetryTest.java

@@ -591,16 +591,48 @@
  }

  /**
   * The walk of a failure looks at a bounded number of links: mssql-jdbc chains every error of one message it
   * received through {@code setNextException}, and a budget spent on those would never reach the cause a wrapper
   * carries. Pinned from both sides - a drop on the last link of the budget is found and one link further is not
   * - since a number nothing pins drifts unnoticed in either direction.
   * The walk of a failure reads every link of it where a decision reads the verdict, however long the chains
   * are - mssql-jdbc chains every error of one message it received through {@code setNextException}. A budget
   * does not leave the question unanswered here: it answers it with the {@code false} of a failure that says
   * nothing about the connection, so the pool is never told of the drop and the attempt is failed to its caller
   * instead of being replayed (issue #961). What terminates the walk is its {@code seen} set, which the cyclic
   * chains of the providers above pin.
   */
  @Test
  public void testTheWalkOfAFailureStopsAtItsBudget()
  public void testTheWalkOfAFailureReachesADropPastAnyBudget()
  {
    assertTrue(JDBCStorage.isConnectionFailure(chainEndingInADrop(64)), "a drop on the last link of the budget");
    assertFalse(JDBCStorage.isConnectionFailure(chainEndingInADrop(65)), "a drop past the budget was walked to");
    assertTrue(JDBCStorage.isConnectionFailure(chainEndingIn(65, "08006")), "a drop one link past the budget");
    assertTrue(JDBCStorage.isConnectionFailure(chainEndingIn(200, "08006")), "a drop far past the budget");
    assertEquals(replayReason(chainEndingIn(200, "08006"), MSSQL, false, false, false),
        "a connection the database dropped", "an attempt worth replaying was failed to its caller");
  }

  /**
   * The line reporting a replay names the link the replay was decided on however deep it sits: a lookup that
   * stopped where the verdict does not would name the statement at the head of the chain instead, and describe
   * a replay that did not happen.
   */
  @Test
  public void testTheSummaryNamesADecidingLinkPastAnyBudget()
  {
    final String drop = conflictSummary(chainEndingIn(200, "08006"), MSSQL);
    assertTrue(drop.contains("08006"), drop);
    final String conflict = conflictSummary(chainEndingIn(200, "40001"), MSSQL);
    assertTrue(conflict.contains("40001"), conflict);
  }

  /**
   * What the budget is left for: the fallback of the summary that names a link no decision reads, where
   * truncation costs the precision of one line of the log. Pinned from both sides - the last link of the budget
   * is named and one link further is not - since a number nothing pins drifts unnoticed in either direction.
   */
  @Test
  public void testTheFallbackOfTheSummaryStopsAtItsBudget()
  {
    assertTrue(conflictSummary(wrappedTimes(63, sql(2627, "23000")), POSTGRES).contains("23000"),
        "the statement on the last link of the budget was not named");
    assertFalse(conflictSummary(wrappedTimes(64, sql(2627, "23000")), POSTGRES).contains("23000"),
        "a link past the budget was walked to");
  }

  /**
@@ -961,6 +993,53 @@
  }

  /**
   * A drop the failure reports past any budget reaches the pool and the replay both: the write is replayed on a
   * connection of its own, and the connections the pool established before the same drop are validated on their
   * next borrow instead of being trusted for the rest of the alive window (issue #961). The driver has not closed
   * the connection here - it is the chains of the failure, and only they, that say the database dropped it.
   */
  @Test
  public void testADropDeepInTheChainReachesThePoolAndTheReplay() throws Exception
  {
    final long window = CachedConnection.aliveBypassNanos;
    CachedConnection.aliveBypassNanos = TimeUnit.HOURS.toNanos(1);
    try
    {
      final Connection pooled = mock(Connection.class);
      when(pooled.isValid(anyInt())).thenReturn(true);
      final Connection borrowed = mock(Connection.class);
      when(borrowed.isValid(anyInt())).thenReturn(true);

      final JDBCStorage storage = storageOver(pooled, borrowed);
      // both are proven alive and back in the pool; the one released last is the one the write borrows
      final Connection first = storage.getConnection();
      final Connection second = storage.getConnection();
      first.close();
      second.close();
      verify(borrowed, never()).isValid(anyInt()); // both are inside the window: nothing has validated

      final AtomicInteger attempts = new AtomicInteger();
      storage.write(txn -> {
        if (attempts.incrementAndGet() == 1)
        {
          // the chain mssql-jdbc reports a message of many errors in, with the one that says the session is
          // gone at the end of it - past the budget the walk used to stop at
          throw new StorageRuntimeException(chainEndingIn(200, "08006"));
        }
      });

      assertEquals(attempts.get(), 2, "a write the database dropped the connection under was not replayed");
      // the borrow of the replay validated rather than trusting the last answer of a connection established
      // before the drop: the pool was told
      verify(borrowed, times(1)).isValid(CachedConnection.VALIDATION_TIMEOUT_SECONDS);
    }
    finally
    {
      CachedConnection.aliveBypassNanos = window;
    }
  }

  /**
   * The create index of the postgres branch is asked of the catalog first, although postgresql has "create index
   * if not exists": that statement commits whether it creates anything or not, and unguarded it would take every
   * write that opens a tree out of the replay - {@code RootContainer.open()} and its ~25 trees per suffix
@@ -1449,18 +1528,29 @@
    return first;
  }

  /** A chain of the given number of next exceptions whose last link is a connection that broke. */
  private static SQLException chainEndingInADrop(int links)
  /** A chain of the given number of next exceptions whose last link carries the given SQLState. */
  private static SQLException chainEndingIn(int links, String lastState)
  {
    final SQLException head = sql(2627, "23000");
    SQLException tail = head;
    for (int link = 2; link <= links; link++)
    {
      tail = chained(tail, sql(0, link == links ? "08006" : "23000")).getNextException();
      tail = chained(tail, sql(0, link == links ? lastState : "23000")).getNextException();
    }
    return head;
  }

  /** The given failure behind the given number of wrappers, the way a caller of this backend wraps one. */
  private static Throwable wrappedTimes(int wrappers, Throwable failure)
  {
    Throwable wrapped = failure;
    for (int wrapper = 0; wrapper < wrappers; wrapper++)
    {
      wrapped = new IllegalStateException("wrapped", wrapped);
    }
    return wrapped;
  }

  /** The second failure suppressed into the first, the way a failing close() joins the failure of an operation. */
  private static SQLException suppressing(SQLException failure, SQLException onRelease)
  {