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

Add engine.Key.Address() method for addressing local keys. #100

Merged
merged 3 commits into from
Oct 3, 2014

Conversation

spencerkimball
Copy link
Member

Local keys are keys which are located on the same range as other keys
minus a local key prefix. Local key prefixes take the form:
"\x00\x00\x00[4 characters]". Any commands addressed to keys of this
form are direct to the range containing the same key minus the prefix.
Transaction records, response cache entries, range metadata (and
in the future others) all have local keys.

Changed cases where we write directly to the underlying engine to use
binary-encoded keys. Now that we binary encode all keys, need to pass
correct prefixes to the C++ custom compaction filter for GC. In this
way, we avoid having C++ need implementations of our decoding methods.

Moved a number of methods into methods of struct engine.Key. Added
associated unittests.

@spencerkimball
Copy link
Member Author

@cockroachdb/developers

Local keys are keys which are located on the same range as other keys
minus a local key prefix. Local key prefixes take the form:
"\x00\x00\x00[4 characters]". Any commands addressed to keys of this
form are direct to the range containing the same key minus the prefix.
Transaction records, response cache entries, range metadata (and
in the future others) all have local keys.

Changed cases where we write directly to the underlying engine to use
binary-encoded keys. Now that we binary encode all keys, need to pass
correct prefixes to the C++ custom compaction filter for GC. In this
way, we avoid having C++ need implementations of our decoding methods.

Moved a number of methods into methods of struct engine.Key. Added
associated unittests.
@spencerkimball spencerkimball force-pushed the spencerkimball/proto-key branch from 9567e1c to 96bc17c Compare October 2, 2014 20:56
}

// MakeLocalKey concatenates KeyLocalPrefix with the specified key
// value, verifying in the process that the length of key is two bytes.
Copy link
Member

Choose a reason for hiding this comment

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

verifying in the process that the prefix has the length of KeyLocalPrefixLength and that this length in bytes is a multiple of seven?

@jiangmingyang
Copy link
Contributor

LGTM

…cally in dist_kv.go and system keys will only be accessible to user root
spencerkimball added a commit that referenced this pull request Oct 3, 2014
Add engine.Key.Address() method for addressing local keys.
@spencerkimball spencerkimball merged commit 0a1e9e0 into master Oct 3, 2014
@spencerkimball spencerkimball deleted the spencerkimball/proto-key branch October 3, 2014 14:18
@@ -18,6 +18,10 @@
#include "rocksdb/c.h"

struct FilterState {
// The key prefix identifying transaction records.
const char* txn_prefix;
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you need to use a char*, length pair here since the local prefix contains \x00 bytes?

Copy link
Member Author

Choose a reason for hiding this comment

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

The \x00 byte in the local prefix is exactly where I want the c++ code to prefix match to, so this was convenient. If I did supply a length here, it'd actually need to be len(prefix)-1 so we didn't actually match against the null character, which won't actually be at that point in the transaction or response cache keys (the null character is the terminator at the end of a binary encoded string).

spencerkimball pushed a commit that referenced this pull request Oct 5, 2014
spencerkimball added a commit that referenced this pull request Oct 5, 2014
bdarnell added a commit to bdarnell/cockroach that referenced this pull request Oct 7, 2014
* origin/master:
  Rework build settings to allow raw "go" tool usage.
  Use curl instead of wget
  Added simple offset measurement between two connected nodes
  Remove and consolidate various uses of reflection.
  Make the build self-contained
  Address Pete and Jiang-Ming's feedback on maximum key length change.
  Addressed Tobias feedback, increased key size limit to 4096
  Enforce key length constraint of 2048 bytes.
  address Tobias' comments
  Address Ben's feedback from PR cockroachdb#100
  replace `go vet` by `go tool vet`: the latter works with files from different packages
  remove double spaces after full stop in comments
  fix some `go vet` errors
  Replace coordinator's naive slice of keys with an interval tree.
  backed out changes to rest.go--this permission check is done automatically in dist_kv.go and system keys will only be accessible to user root
  Updates for Tobias' comments. Added key checking to the REST API.
  Since we're now using random values, we are triggering snappy to disable compression. Make this test more flexible and allow it to assume compression will be disabled due to random value input.
  Add engine.Key.Address() method for addressing local keys.

Conflicts:
	multiraft/clock.go
	multiraft/multiraft.go
	multiraft/storage.go
	multiraft/transport.go
soniabhishek pushed a commit to soniabhishek/cockroach that referenced this pull request Feb 15, 2017
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.

5 participants