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

Merge release to master for 3.1.0 #1761

Merged
merged 199 commits into from
May 15, 2019
Merged

Merge release to master for 3.1.0 #1761

merged 199 commits into from
May 15, 2019

Conversation

abitmore
Copy link
Member

See #1745 for release notes.

windycrypto and others added 30 commits June 7, 2018 14:47
@abitmore abitmore added this to the 3.1.0 - Feature Release milestone May 14, 2019
Copy link
Member

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

looks good.

@jmjatlanta
Copy link
Contributor

The snapshot has a lot of differences. I am attempting to sort through them.

https://gist.github.com/jmjatlanta/8fb91043d7ce1b30b718c93e28754258

The first 290 lines are due to the addition of fail_reason, which is fine. The rest I am attempting to diagnose the cause.

@abitmore
Copy link
Member Author

@jmjatlanta what block numbers did you use for the snapshots? Both fully replayed, or one of them was generated while syncing? That may be the cause of the differences, because 2.7.x objects are impl_transaction_object_type objects which are for "dedupe" which is disabled while replaying.

@jmjatlanta
Copy link
Contributor

@jmjatlanta what block numbers did you use for the snapshots? Both fully replayed, or one of them was generated while syncing? That may be the cause of the differences, because 2.7.x objects are impl_transaction_object_type objects which are for "dedupe" which is disabled while replaying.

That's probably the issue. I picked a very recent block, which probably did some syncing on the first run. I am re-running the first run.

@jmjatlanta
Copy link
Contributor

jmjatlanta commented May 15, 2019

Unfortunately, while improved, the diff still contains many transactions that are different. the first ~290 lines are the fail_reason, but the rest are disturbing. Researching...

Here is the diff: https://gist.github.com/jmjatlanta/14c0376c2c0b7b2c9e2a32b7a782918f

@abitmore
Copy link
Member Author

abitmore commented May 15, 2019

@jmjatlanta transaction_index for dedupe is enabled from head_block - 50. please make sure that your data base for replay is the same.

Update: my fault, should be -50.

@jmjatlanta
Copy link
Contributor

@jmjatlanta transaction_index for dedupe is enabled from head_block - 50. please make sure that your data base for replay is the same.

I am not sure what you mean. Would you provide more information? I am using the same database on each run. I am running the snapshot of the release branch and comparing it to the master branch.

@jmjatlanta
Copy link
Contributor

jmjatlanta commented May 15, 2019

It looks to me as if many (but not all) simply have a different database id. For instance, here are the first few lines of the file (excluding the fail_reason lines), matched by their data, not their id...

old version new version
2.7.5783 none
2.7.8013 none
2.7.48413 none
2.7.131993 2.7.43939
135358 2.7.47304
2.7.149696 2.7.61642

This makes me think that the id fix that was done is the reason for some of the differences, but not all. Also, all the expiration dates for all differences are 5/14 or 5/15, so all expiring yesterday or today, which is curious.

@abitmore
Copy link
Member Author

abitmore commented May 15, 2019

Different IDs means the order they got inserted were different, or different number of transactions inserted before them then got removed. I didn't find why if both were replayed from the same data directory (aka same last_block_num thus same block number that enabled undo_do).

Perhaps should disable p2p connection when replaying to avoid impacts due to syncing after finished replay.

Copy link
Contributor

@jmjatlanta jmjatlanta left a comment

Choose a reason for hiding this comment

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

Tested and snapshot comparison complete.

@jmjatlanta jmjatlanta merged commit e3d1226 into master May 15, 2019
@abitmore
Copy link
Member Author

About the differences on 2.7.x objects:

if( block->timestamp >= last_block->timestamp - gpo.parameters.maximum_time_until_expiration )
skip &= ~skip_transaction_dupe_check;

last_block would change if we let witness_node continue syncing after replay, so, on next replay, skip_transaction_dupe_check flag will be disabled on a later block which will lead to different results.

If we start witness_node with --seed-nodes '[]' parameter, it won't sync after replay, we'll get identical results on next replay.

Alternatively, if we take snapshots on a block older than maximum_time_until_expiration, then there will be no 2.7.x object in snapshots, thus no difference.

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.