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

[fix] Delta ops in TransactionOutput and ChangeSet wrappers #2222

Merged
merged 8 commits into from
Jul 29, 2022

Conversation

georgemitenkov
Copy link
Contributor

@georgemitenkov georgemitenkov commented Jul 26, 2022

Description

This should help to transition from WriteOp::Delta(..) and a number of unreachables into a specialised transaction output and change set local to executor only.

cc @msmouse @gelash. If this approach is ok, DeltaSet will be added and logic to handle deltas (sequential first)

Test Plan


This change is Reviewable

@georgemitenkov georgemitenkov requested review from msmouse and gelash July 26, 2022 15:23
@msmouse
Copy link
Contributor

msmouse commented Jul 26, 2022

Thank you! This IS what I meant. Maybe a more general name like "InternalTransactionOutput", "TransactionOutputExt", etc, in case you guys introduce any other magic.

@georgemitenkov georgemitenkov force-pushed the george/txn-output-with-deltas branch from 5d8e933 to cf0250e Compare July 26, 2022 21:09
@georgemitenkov georgemitenkov marked this pull request as ready for review July 26, 2022 23:49
types/src/delta_set.rs Outdated Show resolved Hide resolved
types/src/delta_set.rs Outdated Show resolved Hide resolved
aptos-move/aptos-vm/src/aptos_vm.rs Outdated Show resolved Hide resolved
aptos-move/aptos-vm/src/aptos_vm.rs Outdated Show resolved Hide resolved
types/src/delta_set.rs Outdated Show resolved Hide resolved
types/src/transaction/mod.rs Outdated Show resolved Hide resolved
@georgemitenkov georgemitenkov force-pushed the george/txn-output-with-deltas branch from 72599dd to dd42d3f Compare July 27, 2022 17:41
@georgemitenkov georgemitenkov force-pushed the george/txn-output-with-deltas branch from 8e8b047 to 4546ad4 Compare July 27, 2022 19:54
@georgemitenkov georgemitenkov changed the title [fix] Draft for using delta-writes in TransactionOutput and ChangeSet [fix] Delta ops in TransactionOutput and ChangeSet wrappers Jul 27, 2022
@georgemitenkov georgemitenkov force-pushed the george/txn-output-with-deltas branch from 4546ad4 to c90dbbe Compare July 27, 2022 23:06
@georgemitenkov georgemitenkov force-pushed the george/txn-output-with-deltas branch from d802e30 to 0c53a9d Compare July 28, 2022 20:51
Copy link
Contributor

@zekun000 zekun000 left a comment

Choose a reason for hiding this comment

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

lgtm, please squash all commits

@georgemitenkov georgemitenkov enabled auto-merge (squash) July 29, 2022 09:18
@georgemitenkov georgemitenkov disabled auto-merge July 29, 2022 09:57
@georgemitenkov georgemitenkov enabled auto-merge (squash) July 29, 2022 10:00
@github-actions
Copy link
Contributor

✅ Forge test success on 9f0fafc477048e67c266e66f05cf56c0f1c7d54e

all up : 4616 TPS, 2387 ms latency, 3150 ms p99 latency,no expired txns

@georgemitenkov
Copy link
Contributor Author

georgemitenkov commented Jul 29, 2022

@zekun000 I think you forgot to approve?

@georgemitenkov georgemitenkov merged commit f3346bb into main Jul 29, 2022
@georgemitenkov georgemitenkov deleted the george/txn-output-with-deltas branch July 29, 2022 17:31
bowenyang007 pushed a commit to bowenyang007/aptos-core that referenced this pull request Jul 29, 2022
…#2222)

Fixes aptos-labs#2057. Now deltas do not escape from executor, and are stored in
`TransactionOutputExt` and `ChangeSetExt` wrappers.
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.

3 participants