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

CockroachDB Physical Backend #2713

Merged
merged 11 commits into from
Jul 23, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 160 additions & 0 deletions physical/cockroachdb.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
package physical

import (
"database/sql"
"fmt"
"sort"
"strings"
"time"

log "github.com/mgutz/logxi/v1"

"github.com/armon/go-metrics"
)

// CockroachDBBackend Backend is a physical backend that stores data
// within a CockroachDB database.
type CockroachDBBackend struct {
table string
client *sql.DB
statements map[string]*sql.Stmt
logger log.Logger
}

// newCockroachDBBackend constructs a CockroachDB backend using the given
// API client, server address, credentials, and database.
func newCockroachDBBackend(conf map[string]string, logger log.Logger) (Backend, error) {
// Get the CockroachDB credentials to perform read/write operations.
connURL, ok := conf["connection_url"]
if !ok || connURL == "" {
return nil, fmt.Errorf("missing connection_url")
}

dbTable, ok := conf["table"]
if !ok {
dbTable = "vault_kv_store"
}

// Create CockroachDB handle for the database.
db, err := sql.Open("postgres", connURL)
if err != nil {
return nil, fmt.Errorf("failed to connect to cockroachdb: %v", err)
}

Copy link
Member

Choose a reason for hiding this comment

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

Can we have a nil check for db here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked this and a *sql.DB will aways be returned when err is nil.

// Create the required table if it doesn't exists.
createQuery := "CREATE TABLE IF NOT EXISTS " + dbTable +
" (path STRING, value BYTES, PRIMARY KEY (path))"
if _, err := db.Exec(createQuery); err != nil {
return nil, fmt.Errorf("failed to create mysql table: %v", err)
}

// Setup the backend
m := &CockroachDBBackend{
table: dbTable,
client: db,
statements: make(map[string]*sql.Stmt),
logger: logger,
}

// Prepare all the statements required
statements := map[string]string{
"put": "INSERT INTO " + dbTable + " VALUES($1, $2)" +
" ON CONFLICT (path) DO " +
" UPDATE SET (path, value) = ($1, $2)",
"get": "SELECT value FROM " + dbTable + " WHERE path = $1",
"delete": "DELETE FROM " + dbTable + " WHERE path = $1",
"list": "SELECT path FROM " + dbTable + " WHERE path LIKE $1",
}
for name, query := range statements {
if err := m.prepare(name, query); err != nil {
return nil, err
}
}
return m, nil
}

// prepare is a helper to prepare a query for future execution
func (m *CockroachDBBackend) prepare(name, query string) error {
stmt, err := m.client.Prepare(query)
if err != nil {
return fmt.Errorf("failed to prepare '%s': %v", name, err)
}
m.statements[name] = stmt
return nil
}

// Put is used to insert or update an entry.
func (m *CockroachDBBackend) Put(entry *Entry) error {
defer metrics.MeasureSince([]string{"cockroachdb", "put"}, time.Now())

_, err := m.statements["put"].Exec(entry.Key, entry.Value)
if err != nil {
return err
}
return nil
}

// Get is used to fetch and entry.
func (m *CockroachDBBackend) Get(key string) (*Entry, error) {
defer metrics.MeasureSince([]string{"cockroachdb", "get"}, time.Now())

var result []byte
err := m.statements["get"].QueryRow(key).Scan(&result)
if err == sql.ErrNoRows {
return nil, nil
}
if err != nil {
return nil, err
}

ent := &Entry{
Key: key,
Value: result,
}
return ent, nil
}

// Delete is used to permanently delete an entry
func (m *CockroachDBBackend) Delete(key string) error {
defer metrics.MeasureSince([]string{"cockroachdb", "delete"}, time.Now())

_, err := m.statements["delete"].Exec(key)
if err != nil {
return err
}
return nil
}

// List is used to list all the keys under a given
// prefix, up to the next prefix.
func (m *CockroachDBBackend) List(prefix string) ([]string, error) {
defer metrics.MeasureSince([]string{"cockroachdb", "list"}, time.Now())

likePrefix := prefix + "%"
rows, err := m.statements["list"].Query(likePrefix)
if err != nil {
return nil, err
}
defer rows.Close()
Copy link
Member

Choose a reason for hiding this comment

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

Should this be done before the err check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you want to do the defer after the error check since rows could be nil.

Copy link
Member

Choose a reason for hiding this comment

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

sounds good


var keys []string
for rows.Next() {
var key string
err = rows.Scan(&key)
if err != nil {
return nil, fmt.Errorf("failed to scan rows: %v", err)
}

key = strings.TrimPrefix(key, prefix)
if i := strings.Index(key, "/"); i == -1 {
// Add objects only from the current 'folder'
keys = append(keys, key)
} else if i != -1 {
// Add truncated 'folder' paths
keys = appendIfMissing(keys, string(key[:i+1]))
}
}

sort.Strings(keys)
Copy link
Member

Choose a reason for hiding this comment

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

This might prove to be expensive if the number of keys being returned are huge. Do we really need to sort them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the way most of the other backends handle the list operation. I wonder if it would be more expensive to do it in the SQL layer or here.

Copy link
Member

Choose a reason for hiding this comment

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

I'll go with what you decide here. Personally, I don't see value in sorting as most of the entries are will be UUIDs.

return keys, nil
}
47 changes: 47 additions & 0 deletions physical/cockroachdb_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package physical

import (
"os"
"testing"

"github.com/hashicorp/vault/helper/logformat"
log "github.com/mgutz/logxi/v1"

_ "github.com/lib/pq"
)

func TestCockroachDBBackend(t *testing.T) {
connURL := os.Getenv("CRURL")
if connURL == "" {
t.SkipNow()
}

table := os.Getenv("CRTABLE")
if table == "" {
table = "vault_kv_store"
}

// Run vault tests
logger := logformat.NewVaultLogger(log.LevelTrace)

b, err := NewBackend("cockroachdb", logger, map[string]string{
"connection_url": connURL,
"table": table,
})

if err != nil {
t.Fatalf("Failed to create new backend: %v", err)
}

defer func() {
crdb := b.(*CockroachDBBackend)
_, err := crdb.client.Exec("TRUNCATE TABLE " + crdb.table)
if err != nil {
t.Fatalf("Failed to drop table: %v", err)
}
}()

testBackend(t, b)
testBackend_ListPrefix(t, b)

}
1 change: 1 addition & 0 deletions physical/physical.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ var builtinBackends = map[string]Factory{
"mssql": newMsSQLBackend,
"mysql": newMySQLBackend,
"postgresql": newPostgreSQLBackend,
"cockroachdb": newCockroachDBBackend,
"swift": newSwiftBackend,
"gcs": newGCSBackend,
}
Expand Down