Skip to content
This repository was archived by the owner on Aug 23, 2020. It is now read-only.

Feat: Added an option to fast forward the ledger state#1196

Merged
GalRogozinski merged 11 commits intoiotaledger-archive:dev-localsnapshotsfrom
iotadevelopment:merge_fastforward
Nov 29, 2018
Merged

Feat: Added an option to fast forward the ledger state#1196
GalRogozinski merged 11 commits intoiotaledger-archive:dev-localsnapshotsfrom
iotadevelopment:merge_fastforward

Conversation

@hmoog
Copy link
Copy Markdown

@hmoog hmoog commented Nov 26, 2018

Description

This PR introduces a way to fast forward the ledger state after a restart of IRI. Previously we had to rescan all milestones to rebuild the ledger state. Especially for nodes that start off with a non-snapshotted database, this could take a really long time (a few hours). In combination with the upcoming refactor of the replayMilestones method this allows us to massively decrease the time and resources that are required for this operation (to a few seconds rather than hours).

Type of change

  • Enhancement (a non-breaking change which adds functionality)

Checklist:

  • My code follows the style guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@iotaledger-archive iotaledger-archive deleted a comment Nov 27, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 27, 2018
@iotaledger-archive iotaledger-archive deleted a comment from hmoog Nov 27, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 28, 2018
Copy link
Copy Markdown
Contributor

@GalRogozinski GalRogozinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good!

Liked your mock utils :-)

Only one change request...


@Before
public void setUp() {
SnapshotMockUtils.mockSnapshotProvider(snapshotProvider);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not put this in the constructor?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the mocked properties are NULL in the constructor.

public void setUp() {
SnapshotMockUtils.mockSnapshotProvider(snapshotProvider);

MilestoneViewModel.clear();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be in After?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other tests don't clean up the cached MilestoneViewModels so it can happen that we start this test with some cache Milestones, but you are right this should ALSO go into after the test.

public class SnapshotMockUtils {
private static final int DEFAULT_MILESTONE_START_INDEX = 70000;

private static final Hash DEFAULT_GENESIS_HASH = Hash.NULL_HASH;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use Hash.NULL_HASH directly?

Copy link
Copy Markdown
Author

@hmoog hmoog Nov 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make it easier to reason about the logic and allow the "user" of this utils class to easily determine the important values for the mocked objects.

Otherwise anybody who uses this class would have to examine the code of the specific method to know which values are used for the mocked objects.

I would even consider making them public so tests can check against these values without reading them from the mocked objects (that might for example be usefull when testing the rollback functionality - atm I read them from the mocked object and assume that the value was not magically modified during any other call).

But this will be refined ans grow as I add more tests.

Copy link
Copy Markdown
Contributor

@alon-e alon-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@iotaledger-archive iotaledger-archive deleted a comment Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment from hmoog Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment from hmoog Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment from hmoog Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment from hmoog Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment from hmoog Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment from hmoog Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment from hmoog Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment from hmoog Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment from hmoog Nov 29, 2018
@iotaledger-archive iotaledger-archive deleted a comment from hmoog Nov 29, 2018
Optional<MilestoneViewModel> milestone = milestoneService.findLatestProcessedSolidMilestoneInDatabase();
if (milestone.isPresent()) {
snapshotService.replayMilestones(snapshotProvider.getLatestSnapshot(), milestone.get().index());
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks nicer :-)

@GalRogozinski GalRogozinski merged commit fec0935 into iotaledger-archive:dev-localsnapshots Nov 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants