bookkeeper icon indicating copy to clipboard operation
bookkeeper copied to clipboard
verified publisher icon apache

(WIP) Auditor.checkAllLedgers should use Auditor's instance variables

Open reddycharan opened this issue 7 years ago • 8 comments

Descriptions of the changes in this PR:

  • currently checkAllLedgers method creates instances of ZK, BK and BKAdmin everytime this method is called. Instead it should use Auditors instance variables.

reddycharan avatar Aug 08 '18 19:08 reddycharan

retest this please

reddycharan avatar Aug 09 '18 05:08 reddycharan

few replication tests are failing. Will check that.

reddycharan avatar Aug 09 '18 17:08 reddycharan

the change here will introduce deadlock so this PR needs to be revisited.

See comment: https://github.com/apache/bookkeeper/pull/1608#issuecomment-414957226

sijie avatar Aug 22 '18 08:08 sijie

@reddycharan this patch got enough consensus to be committed. can you please rebase ? I will be happy to merge

cc @sijie

eolivelli avatar Sep 22 '19 12:09 eolivelli

Oh I missed @sijie 's comment. we should revisit this patch or close it

eolivelli avatar Sep 22 '19 12:09 eolivelli

@reddycharan this patch was already approved by @sijie

do you mind rebasing it to current master ?

eolivelli avatar Jun 12 '20 15:06 eolivelli

@reddycharan can you please rebase to master and push? This is a useful patch to have.

Ghatage avatar Oct 28 '20 20:10 Ghatage

@eolivelli @sijie I went through the current implementation of checkAllLedgers() and I think we should close this PR.

Ghatage avatar Jul 12 '24 06:07 Ghatage