[fix][ml] Topic load timeout due to ml data ledger future never finishes

[poorbarcode]

Motivation

Background
There is a mechanism that repeatedly prevents the callback of ML data ledger creation:

• Start a scheduled task to check whether the creation will be timeout.
• Received a callback
  • Check whether the future(@param ctx of BK.createAsync) has been done or not.
  • If done: it means the creation has timeout before the creation is completed
  • Otherwise: it is a real callback from BK.

Issue:
But the timeout event will call the same callback as above, then the steps are as follows, which you ca reproduce by the test testCreateDataLedgerTimeout:

• Start creating a data ledger
  • Call BK.createAsync
• Timeout
  • Mark the future(@param ctx of BK.createAsync) as completed exceptionally.
  • Trigger the callback related to ledger creation.
    • Check whether the future(@param ctx of BK.createAsync) has been done or not.
    • If done: do nothing.
• Creation is compelled.
  • Trigger the callback related to ledger creation.
    • Check whether the future(@param ctx of BK.createAsync) has been done or not.
    • If done: do nothing.
• Issue: The callback for ledger creation will never be called.

Modifications

Fix the issue

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository: x

[poorbarcode]

/pulsarbot rerun-failure-checks

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.19%. Comparing base (bbc6224) to head (37d066c).
Report is 813 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #23772      +/-   ##
============================================
+ Coverage     73.57%   74.19%   +0.62%     
+ Complexity    32624    32159     -465     
============================================
  Files          1877     1853      -24     
  Lines        139502   143373    +3871     
  Branches      15299    16276     +977     
============================================
+ Hits         102638   106382    +3744     
+ Misses        28908    28624     -284     
- Partials       7956     8367     +411     

Flag	Coverage Δ
inttests	26.71% <50.00%> (+2.13%)	⬆️
systests	23.10% <0.00%> (-1.22%)	⬇️
unittests	73.73% <100.00%> (+0.88%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...che/bookkeeper/mledger/impl/ManagedLedgerImpl.java	81.85% <100.00%> (+1.19%)	⬆️

... and 1021 files with indirect coverage changes

[nodece]

It looks you should fix the checkAndCompleteLedgerOpTask():

    protected boolean checkAndCompleteLedgerOpTask(int rc, LedgerHandle lh, Object ctx) {
        if (ctx instanceof CompletableFuture) {
            // ledger-creation is already timed out and callback is already completed so, delete this ledger and return.
            if (((CompletableFuture) ctx).complete(lh)) {
                return false;
            } else {
                if (rc == BKException.Code.OK) {
                    log.warn("[{}]-{} ledger creation timed-out, deleting ledger", this.name, lh.getId());
                    asyncDeleteLedger(lh.getId(), DEFAULT_LEDGER_DELETE_RETRIES);
                    return true; // changed
                }
                return false; // changed
            }
        }
        return false;
    }

[shibd]

It looks you should fix the checkAndCompleteLedgerOpTask():

+1

[poorbarcode]

@shibd @nodece

It looks you should fix the checkAndCompleteLedgerOpTask():

Good suggestion. Modified.

[dao-jun]

It looks you should fix the checkAndCompleteLedgerOpTask():

    protected boolean checkAndCompleteLedgerOpTask(int rc, LedgerHandle lh, Object ctx) {
        if (ctx instanceof CompletableFuture) {
            // ledger-creation is already timed out and callback is already completed so, delete this ledger and return.
            if (((CompletableFuture) ctx).complete(lh)) {
                return false;
            } else {
                if (rc == BKException.Code.OK) {
                    log.warn("[{}]-{} ledger creation timed-out, deleting ledger", this.name, lh.getId());
                    asyncDeleteLedger(lh.getId(), DEFAULT_LEDGER_DELETE_RETRIES);
                    return true; // changed
                }
                return false; // changed
            }
        }
        return false;
    }

It seems no need to change here, the following steps will handle the case of rc != OK