Skip to content

db: add txn discard to api#437

Merged
sanderpick merged 1 commit intomasterfrom
sander/expose-txn-discard
Sep 16, 2020
Merged

db: add txn discard to api#437
sanderpick merged 1 commit intomasterfrom
sander/expose-txn-discard

Conversation

@sanderpick
Copy link
Copy Markdown
Contributor

Signed-off-by: Sander Pick sanderpick@gmail.com

Signed-off-by: Sander Pick <sanderpick@gmail.com>
@sanderpick sanderpick self-assigned this Sep 16, 2020
@sanderpick sanderpick requested a review from asutula September 16, 2020 18:25
t.Fatalf("failed to write txn discard: %v", err)
}

err = txn.Save(existingPerson)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To be clear, this method discards the changes, but it doesn't abort the transaction itself, correct? To abort the transaction, it looks like we need to close the context, is that right?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I ask because from a remote client perspective, this doesn't appear to be possible?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, exactly, it's still "alive" but will error when it's committed using End

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perfect!

t.Fatalf("failed to start write txn: %v", err)
}
defer func() {
err = end()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@carsonfarmer this is calling the func returned by the Go client that actually just sends a CloseSend to the server, which makes it release the transaction... that means it will be committed, but will error if it has been marked as "discarded"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You can see how the Go client "ends" the transaction here: https://github.com/textileio/go-threads/blob/master/api/client/write.go#L233 (note the extra error catch...)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeh this is exactly what I needed. Perfect. One thing I noticed though, in the js client, we're not properly getting the final response as in here: https://github.com/textileio/go-threads/blob/master/api/client/write.go#L237 which we need to ensure the transaction was successful or not.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So this has been a useful thread/discussion. Cheers!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah totally, I ran into that yesterday: #436 (comment)

@sanderpick sanderpick merged commit 5d3f3d5 into master Sep 16, 2020
@sanderpick sanderpick deleted the sander/expose-txn-discard branch September 16, 2020 18:46
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.

2 participants