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

Design doc updates #9646

Merged
merged 2 commits into from
Sep 30, 2016
Merged

Design doc updates #9646

merged 2 commits into from
Sep 30, 2016

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Sep 30, 2016

This change is Reviewable

Copy link
Collaborator

@petermattis petermattis left a comment

Choose a reason for hiding this comment

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

LGTM


- `\0\0meta1` Range metadata for location of `\0\0meta2`.
- `\0\0meta1<key1>` Range metadata for location of `\0\0meta2<key1>`.
- `\x02<key1>`: Range metadata for range ending `\x03<key1>`.
Copy link
Collaborator

@petermattis petermattis Sep 30, 2016

Choose a reason for hiding this comment

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

I think it is worth mentioning this is a meta1 key. Ditto for the \x03 and \x04 prefix below.

- `\x03<keyN>`: Range metadata for range ending `<keyN>`.
- `\x04{desc,node,range,store}-idegen`: ID generation oracles for various component types.
- `\x04status-node-<varint encoded Store ID>`: Store runtime metadata.
- `\x04tsd<...>`: Time-series data.
- `<key0>`: `<value0>` The first user data key.**
Copy link
Collaborator

@petermattis petermattis Sep 30, 2016

Choose a reason for hiding this comment

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

user data key is mildly confusing. Perhaps table data key.

@@ -209,27 +209,27 @@ on a node is at a timestamp < next HLC time.

Transactions are executed in two phases:

1. Start the transaction by writing a new entry to the system
transaction table (keys prefixed by *\0tx*) with state “PENDING”. In
1. Start the transaction by selecting a range which is likely to be
Copy link
Member

Choose a reason for hiding this comment

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

s/is likely to be heavily involved in the transaction/involved in the first write of the transaction/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why? Isn't this what we just discussed as being the more-likely-to-rot variant?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, agree that this is too specific.

@@ -267,7 +267,7 @@ encounter data that necessitate conflict resolution

When a transaction restarts, it changes its priority and/or moves its
timestamp forward depending on data tied to the conflict, and
begins anew reusing the same tx id. The prior run of the transaction might
begins anew reusing the same txn id. The prior run of the transaction might
have written some write intents, which need to be deleted before the
transaction commits, so as to not be included as part of the transaction.
Copy link
Member

Choose a reason for hiding this comment

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

Not correct, epochs take care of this.

Copy link
Member

Choose a reason for hiding this comment

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

The whole section from here down is bogus.

@@ -267,7 +267,7 @@ encounter data that necessitate conflict resolution

When a transaction restarts, it changes its priority and/or moves its
timestamp forward depending on data tied to the conflict, and
begins anew reusing the same tx id. The prior run of the transaction might
begins anew reusing the same txn id. The prior run of the transaction might
have written some write intents, which need to be deleted before the
transaction commits, so as to not be included as part of the transaction.
Copy link
Member

Choose a reason for hiding this comment

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

The whole section from here down is bogus.

clean up after itself. The next attempt (if applicable) then runs as a
new transaction with **a new tx id**.
record, finds that it has been aborted. In this case, the transaction
can not reuse its intents; it returns control to the client before
Copy link
Member

Choose a reason for hiding this comment

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

It never really reuses intents. The intents' primary role is that of preventing starvation by forcing conflicts.

dangling intents as they encounter them) but will make an effort to
clean up after itself. The next attempt (if applicable) then runs as a
new transaction with **a new tx id**.
record, finds that it has been aborted. In this case, the transaction
Copy link
Member

Choose a reason for hiding this comment

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

s/upon/usually upon/ (because abort span, etc)

- `\x03<keyN>`: Range metadata for range ending `<keyN>`. This a "meta2" key.
- `\x04{desc,node,range,store}-idegen`: ID generation oracles for various component types.
- `\x04status-node-<varint encoded Store ID>`: Store runtime metadata.
- `\x04tsd<key>`: Time-series data key.
Copy link
Member

Choose a reason for hiding this comment

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

Where did zones go? Are they in one of the magic tables?

@tamird
Copy link
Contributor Author

tamird commented Sep 30, 2016

@tschottdorf can the lies which I re-wrapped be addressed subsequently? Zones are now in a SQL table.

@tbg
Copy link
Member

tbg commented Sep 30, 2016

Sure.

@tamird tamird merged commit 83fd19a into cockroachdb:master Sep 30, 2016
@tamird tamird deleted the design-doc-update branch September 30, 2016 21:02
@jseldess jseldess mentioned this pull request Oct 5, 2016
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.

4 participants