Skip to content

Conversation

@jvdp
Copy link
Contributor

@jvdp jvdp commented May 28, 2021

NB. this is based off of #977

This PR includes:

  • improved (debug) logging in FastSync to help diagnose issues
  • add (unique) names for every actor (for the benefit of logging)
  • fix edge cases where Terminated message wasn't handled and FastSync was stopped
  • fix bug where skeleton handler actor failure wouldn't be recovered from, causing fast sync to get stuck (currentSkeleton and skeletonHandler are now cleared.)
  • fix issue(s) caused by stale state / actors hanging around after branch resolution (every child actor is now killed and a new SyncingHandler is instantiated.)

@leo-bogastry
Copy link
Contributor

So one thing I have experienced when synching with Astor, was the fastSync failing without a proper error.
Here https://github.com/input-output-hk/mantis/pull/1001/files#diff-4b491aec4f7ba3bc8091f81d49100e0ccc38a2f396bcca38434118637e6c0befL637 when it calls validateBlocks, that method just returns Valid or Invalid.
So what do you think of also adding a log there?
This was my modification

def validateBlocks(
      requestedHashes: Seq[ByteString],
      blockBodies: Seq[BlockBody],
      log: LoggingAdapter
  ): BlockBodyValidationResult = {
    var result: BlockBodyValidationResult = Valid
    (requestedHashes zip blockBodies)
      .map { case (hash, body) => (blockchain.getBlockHeaderByHash(hash), body) }
      .forall {
        case (Some(header), body) =>
          val validationResult: Either[StdBlockValidator.BlockError, BlockValid] =
            validators.blockValidator.validateHeaderAndBody(header, body)
          result = validationResult.fold(
            error => {
              log.error("Block body validation failed with error {}", error)
              Invalid
            },
            _ => Valid
          )
          validationResult.isRight
        case _ =>
          result = DbError
          false
      }
    result
  }

@jvdp
Copy link
Contributor Author

jvdp commented Jun 1, 2021

@LeonorLunatech thanks for the suggestion, I added the log message and also refactored it a bit.

@jvdp jvdp merged commit e64b102 into develop Jun 1, 2021
@jvdp jvdp deleted the bug/ETCM-819 branch June 1, 2021 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants