Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sideband information to database. #1554

Merged
merged 7 commits into from Jan 27, 2019

Conversation

Projects
6 participants
@clemahieu
Copy link
Collaborator

commented Jan 4, 2019

This commit adds block-type-specific sideband information which eliminates expensive value computations.
This information is stored in-line with the block itself, instead of in a separate table, for efficient retrieval.
Since this involves rewriting all block entries the process is run iteratively in the background.
Once the upgrade is complete the blocks_info table is dropped from the database.

@clemahieu clemahieu requested review from argakiig and SergiySW Jan 4, 2019

@SergiySW SergiySW added the major label Jan 5, 2019

@rkeene rkeene added the enhancement label Jan 7, 2019

@zhyatt zhyatt added this to the V18.0 milestone Jan 8, 2019

@zhyatt zhyatt added this to CP 2 (2018-01-16) in V18 Jan 8, 2019

clemahieu and others added some commits Jan 2, 2019

Add sideband information to database.
This commit adds block-type-specific sideband information which eliminates expensive value computations.
This information is stored in-line with the block itself, instead of in a separate table, for efficient retrieval.
Since this involves rewriting all block entries the process is run iteratively in the background.
Once the upgrade is complete the blocks_info table is dropped from the database.
Sideband RPC & open blocks adjustment (#1555)
* Disable sideband account & height for open blocks

* Add sideband information to RPC block_info

* Replacing RPC block with PRC block_info

* Add local timestamp to RPC account_history

* Fix

* Rolling back changes from different PR

* Correct account & balance with sideband

* Update rpc.blocks_info test
Improving log message for sideband upgrade process.
Initializing timestamp to sentinal value during the upgrade process.

@clemahieu clemahieu force-pushed the block_sideband_squash branch from 96d1b8f to 99eca06 Jan 9, 2019

@argakiig
Copy link
Collaborator

left a comment

needs rebase but will be good afterwards

sideband_a->balance = block_balance_computed (transaction_a, hash_a);
sideband_a->successor = block_successor (transaction_a, hash_a);
sideband_a->height = std::numeric_limits<uint64_t>::max ();
sideband_a->timestamp = std::numeric_limits<uint64_t>::max ();

This comment has been minimized.

Copy link
@SergiySW

SergiySW Jan 11, 2019

Collaborator

May be set default timestampt to 0?

This comment has been minimized.

Copy link
@clemahieu

clemahieu Jan 14, 2019

Author Collaborator

Does anyone else have an opinion on it? I used height = max as a sentinel because 0 is a valid height. I could set the timestamp sentinel to 0 instead of max, this just matches height.

This comment has been minimized.

Copy link
@SergiySW

This comment has been minimized.

Copy link
@rkeene

rkeene Jan 25, 2019

Contributor

Either is fine with me. 0 is falsey, which may be useful in some contexts, it's more commonly used to represent "time unavailable", and it's also easier to type. ::max() will never be a valid time, even if an unreasonable epoch is used for the system time keeping, and it's consistent with the existing members.

@cryptocode

This comment has been minimized.

Copy link
Collaborator

commented Jan 13, 2019

I noticed the new tests use iterations - does the deadline approach not fit in this case? Not that it matters much, just wondering if deadline_set needs improvement.

@clemahieu

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 14, 2019

I noticed the new tests use iterations - does the deadline approach not fit in this case? Not that it matters much, just wondering if deadline_set needs improvement.

This doesn't use system.poll, would it make sense to implement it here?

@cryptocode

This comment has been minimized.

Copy link
Collaborator

commented Jan 14, 2019

Ah, there's no nano::system involved here. Could add freestanding deadline support one day to keep things consistent, but not important.

@SergiySW
Copy link
Collaborator

left a comment

Other than default timestamp LGTM
Made several days runs on live/beta networks in Debug mode, seems stable

clemahieu added some commits Jan 27, 2019

Merge branch 'master' into block_sideband_squash
# Conflicts:
#	nano/lib/interface.h
#	nano/node/lmdb.cpp
#	nano/secure/ledger.cpp
Merge branch 'master' into block_sideband_squash
# Conflicts:
#	nano/node/rpc.cpp

@clemahieu clemahieu merged commit 5dfe580 into master Jan 27, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@zhyatt zhyatt referenced this pull request Jan 28, 2019

Closed

Block sideband #1523

@clemahieu clemahieu deleted the block_sideband_squash branch Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.