Add commit marker with timestamp #9266
Conversation
|
This pull request was exported from Phabricator. Differential Revision: D31721350 |
|
This pull request was exported from Phabricator. Differential Revision: D31721350 |
riversand963
force-pushed
the
export-D31721350
branch
from
7e3be40 to
b7d5410
Compare
|
This pull request was exported from Phabricator. Differential Revision: D31721350 |
riversand963
force-pushed
the
export-D31721350
branch
from
b7d5410 to
53113c2
Compare
|
This pull request was exported from Phabricator. Differential Revision: D31721350 |
riversand963
force-pushed
the
export-D31721350
branch
from
53113c2 to
727fb4c
Compare
|
This pull request was exported from Phabricator. Differential Revision: D31721350 |
riversand963
force-pushed
the
export-D31721350
branch
from
727fb4c to
941622a
Compare
|
This pull request was exported from Phabricator. Differential Revision: D31721350 |
riversand963
force-pushed
the
export-D31721350
branch
from
941622a to
33d5495
Compare
riversand963
force-pushed
the
export-D31721350
branch
from
33d5495 to
9f7ec0c
Compare
|
This pull request was exported from Phabricator. Differential Revision: D31721350 |
1 similar comment
|
This pull request was exported from Phabricator. Differential Revision: D31721350 |
riversand963
force-pushed
the
export-D31721350
branch
from
9f7ec0c to
7be487b
Compare
| @@ -2114,6 +2149,76 @@ class MemTableInserter : public WriteBatch::Handler { | |||
| return s; | |||
| } | |||
|
|
|||
| Status MarkCommitWithTimestamp(const Slice& name, | |||
There was a problem hiding this comment.
I think it would be nice to reuse/share some code between MarkCommit and MarkCommitWithTimestamp.
There was a problem hiding this comment.
I am thinking of keeping this mid-sized PR simple such that it does not change existing logic used intensively by production workloads at this sensitive time of year :).
I am also not very sure if future changes to timestamp/transaction will make these two methods' implementation diverge for more complex transaction write-policies, e.g. write-prepared/write-unprepared. Maybe it's not the worst idea just to leave them separate for now.
|
Thanks for the PR @riversand963 ! LGTM in general, just one comment. P.S. Also, there are a couple of Java failures that seem legit |
Summary: Pull Request resolved: facebook#9266 This diff adds a new tag `CommitWithTimestamp`. Currently, there is no API to trigger writing this tag to WAL, thus it is unavailable to users. This is an ongoing effort to add user-defined timestamp support to write-committed transactions. This diff also indicates all column families that may potentially participate in the same transaction must either disable timestamp or have the same timestamp format, since `CommitWithTimestamp` tag is followed by a single byte-array denoting the commit timestamp of the transaction. We will enforce this checking in a future diff. We keep this diff small. Reviewed By: ltamasi Differential Revision: D31721350 fbshipit-source-id: 16636d7441d79eec3cda73bff4b699f545e7aeab
|
This pull request was exported from Phabricator. Differential Revision: D31721350 |
riversand963
force-pushed
the
export-D31721350
branch
from
7be487b to
f087f42
Compare
|
Thanks @ltamasi for the review! |
Summary:
This diff adds a new tag
CommitWithTimestamp. Currently, there is no API to trigger writingthis tag to WAL, thus it is unavailable to users.
This is an ongoing effort to add user-defined timestamp support to write-committed transactions.
This diff also indicates all column families that may potentially participate in the same
transaction must either disable timestamp or have the same timestamp format, since
CommitWithTimestamptag is followed by a single byte-array denoting the committimestamp of the transaction. We will enforce this checking in a future diff. We keep this
diff small.
Differential Revision: D31721350