/pulsarbot rerun-failure-checks

Besides, I remember that ZK cannot remove recursively at a batch but iterating from children to parent. How the cache won't be invalidated by its children getting deleted?

Anyway, invalidate cache eagerly won't introduce correctness issue.

Besides, I remember that ZK cannot remove recursively at a batch but iterating from children to parent. How the cache won't be invalidated by its children getting deleted?

Anyway, invalidate cache eagerly won't introduce correctness issue.

In the scenario described in #14779 (comment), node1 does not have a watcher associated with the newly created znode cursor. As a result, the mechanism to invalidate the parent key children cache will not work.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.95%. Comparing base (1e664b7) to head (4e9086f).
⚠️ Report is 2082 commits behind head on master.

@@              Coverage Diff              @@
##             master   #20363       +/-   ##
=============================================
+ Coverage     36.93%   72.95%   +36.01%     
- Complexity    12032    31889    +19857     
=============================================
  Files          1687     1864      +177     
  Lines        128761   138403     +9642     
  Branches      14002    15185     +1183     
=============================================
+ Hits          47553   100965    +53412     
+ Misses        74932    29427    -45505     
- Partials       6276     8011     +1735     

Thanks for your explanation! Now I know that no children cause the cache be empty and return empty cursor even if there is some. And this patch is valid even for defensive programming (delete node should always invalid cache).

Although, if the cursor is a child of the topic znode, won't putInternal invalid childrenCache so that the cursor be properly loaded?

I'd appreciate it if you can provide a znode view of execution steps to demonstrate the exception.

I'd appreciate it if you can provide a znode view of execution steps to demonstrate the exception.

Try me break down into two steps to explain how this error could be produced in #14779:

1. When topic back to node1, it will only initialize the managed-ledger without initializing any cursors, due to cache consistency
   pseudo call stack:

MetaStore#getCursor
ManagedLedgerImpl#initializeCursors
MetaStore#asyncUpdateLedgerIds
ManagedLedgerImpl#asyncCreateLedger
ManagedLedgerImpl#initializeBookKeeper
MetaStore#getManagedLedgerInfo
ManagedLedgerImpl#initialize

2. When a consumer reconnects to node1, the broker only checks if the cursor has been loaded into memory
   

   pulsar/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java

   Lines 981 to 982 in 1e664b7

   	ManagedCursor cachedCursor = cursors.get(cursorName);
   	if (cachedCursor != null) {

pseudo call stack:

AbstractMetadataStore#put
MetaStore#asyncUpdateCursorInfo
ManagedCursorImpl#persistPositionMetaStore
ManagedCursorImpl#switchToNewLedger
ManagedCursorImpl#createNewMetadataLedger
ManagedCursorImpl#initialize
ManagedLedger#asyncOpenCursor
PersistentTopic#getDurableSubscription
PersistentTopic#internalSubscribe
PersistentTopic#subscribe
ServerCnx#handleSubscribe

BTW, broker will try create a cursor znode which already exists in zk, so it'll receive NODEEXISTS.

Here's NODEEXISTS how to be handled, a BADVERSION:

In summary, this is how I observed the occurrence of BadVersionException.

After a discussion with @Shawyeok I learned that it's about two broker node that:

1. Broker 1 has cache for topic znode.
2. Broker 1 switches to a different topic, delete the topic znode but forget to clear the children cache.
3. Broker 2 create the topic znode and cursor child znode
4. Broker 1 switches back to the original topic. It doesn't set watches or perform put cursor znode by itself so the child cache is still empty (not uninit).

Merging...

Thanks for your contribution and detailed investigation @Shawyeok!

@Shawyeok Could you add a test for this patch?

@Shawyeok Could you add a test for this patch?

Sure, I'll do it later.

This patch was aimed to fix #14779, I reproduced in my environment which is based on the 2.8.1 codebase. After following @Technoboy's suggestion to write a reproduction test in the master branch, I realized that #11198 uses zk persistent watches, which should also fix #14779.

However, in theory, there is a chance that the new cursor watcher event is not triggered before MetadataStore#getChildren gets called on other brokers. In such cases, this patch also works effectively.

As discussed on the mailing list https://lists.apache.org/thread/w4jzk27qhtosgsz7l9bmhf1t7o9mxjhp, there is no plan to release 2.9.6, so I am going to remove the release/2.9.6 label