wenbingshen commented on code in PR #3833:
URL: https://github.com/apache/bookkeeper/pull/3833#discussion_r1184622195
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerFragment.java:
##########
@@ -229,15 +229,16 @@ public void setReplicateType(ReplicateType replicateType)
{
@Override
public String toString() {
return String.format("Fragment(LedgerID: %d, FirstEntryID: %d[%d], "
- + "LastKnownEntryID: %d[%d], Host: %s, Closed: %s)", ledgerId,
firstEntryId,
+ + "LastKnownEntryID: %d[%d], Host: %s, Closed: %s, Type: %s)",
ledgerId, firstEntryId,
getFirstStoredEntryId(), lastKnownEntryId,
getLastStoredEntryId(),
- getAddresses(), isLedgerClosed);
+ getAddresses(), isLedgerClosed, replicateType);
}
/**
* ReplicateType.
*/
public enum ReplicateType {
+ NULL,
Review Comment:
@hangc0276 @horizonzy Yes, when I first looked at the use of LedgerFragment,
I also thought it only had two states, `DATA_LOSS` or
`DATA_NOT_ADHERING_PLACEMENT`, when I read the code again, I felt something was
different.
`LedgerFragmentReplicator` is used to copy and restore those copies of a
ledger segment in a failed bookie. This state initialization is `DATA_LOSS`.
`ReplicationWorker` will first recover the LedgerFragment of `DATA_LOSS`
state, and then recover the LedgerFragment of `DATA_NOT_ADHERING_PLACEMENT`.
When `BookKeeperAdmin` is recovering Ledger, LedgerFragment is in
`DATA_LOSS` state.
The above LedgerFragment is mainly in recover or replicate, and
LedgerFragment has an obvious state.
In addition to the above, there is also a LedgerFragment, which is in an
unclear state and mainly occurs inside `LedgerChecker`.
`AuditorCheckAllLedgersTask` and `ReplicationWorker` use `LedgerChecker`
checkLedger. By default, it will check the first and last Entry of each
LedgerFragment of the ledger before the check is completed. , LedgerFragment
should not be in `DATA_LOSS` or `DATA_NOT_ADHERING_PLACEMENT`, because after
the inspection is completed, the LedgerFragment may be normal, or the replicate
may be missing; before and after the inspection is completed, we sometimes
print the LedgerFragment through debug, which may cause a wrong state, so I
added the `NULL` state is used to identify this situation.
https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java#L476-L492
```java
for (LedgerFragment r : fragments) {
if (LOG.isDebugEnabled()) {
LOG.debug("Checking fragment {}", r);
}
try {
verifyLedgerFragment(r, allFragmentsCb,
percentageOfLedgerFragmentToBeVerified);
} catch (InvalidFragmentException ife) {
LOG.error("Invalid fragment found : {}", r);
allFragmentsCb.operationComplete(
BKException.Code.IncorrectParameterException, r);
} catch (BKException e) {
LOG.error("BKException when checking fragment : {}", r, e);
} catch (InterruptedException e) {
LOG.error("InterruptedException when checking fragment :
{}", r, e);
}
}
```
`NULL` may be difficult to understand, maybe we can add `INIT` before
checking, and `NORMAL` state if the segment not data loss after checking?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
