Skip to content

Conversation

@jzhou77
Copy link
Contributor

@jzhou77 jzhou77 commented Jun 19, 2019

This allows client to set transaction size as a database option, or as a transaction option.

This fixes #1681.

@jzhou77 jzhou77 marked this pull request as ready for review June 20, 2019 06:31
@jzhou77 jzhou77 requested a review from alecgrieser June 20, 2019 06:31
@alecgrieser alecgrieser self-assigned this Jun 20, 2019
Copy link
Contributor

@alecgrieser alecgrieser left a comment

Choose a reason for hiding this comment

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

This looks good overall, with just a few things. Were you able to verify this by trying to write a transaction that was larger than same custom option but still less than 10 MB and getting a failure?

@jzhou77
Copy link
Contributor Author

jzhou77 commented Jun 20, 2019

I've addressed all comments and added a python test that verified the size limit is effective. The test catches a bug of not setting per transaction limit, which was fixed.

Copy link
Contributor

@alecgrieser alecgrieser left a comment

Choose a reason for hiding this comment

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

Code looks good. I had two more comments on the docs that I left in the form of change suggestions.

@alecgrieser alecgrieser merged commit e8c7550 into apple:master Jun 21, 2019
@jzhou77 jzhou77 added this to the 6.2 milestone Jul 1, 2019
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.

Create a transaction option for limiting the transaction size below 10 MB

2 participants