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

Minor fixes in the way of init & start #16

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ _testmain.go
.vagrant

cockroach
*.swp
12 changes: 8 additions & 4 deletions server/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
package server

import (
"flag"

commander "code.google.com/p/go-commander"
"code.google.com/p/go-uuid/uuid"
"github.com/cockroachdb/cockroach/storage"
Expand Down Expand Up @@ -46,7 +48,9 @@ to the correct level upon start.

To start the cluster after initialization, run "cockroach start".
`,
Run: runInit}
Run: runInit,
Flag: *flag.CommandLine,
}

// runInit.
func runInit(cmd *commander.Command, args []string) {
Expand All @@ -57,12 +61,12 @@ func runInit(cmd *commander.Command, args []string) {
// Specifying the disk type as HDD may be incorrect, but doesn't
// matter for this bootstrap step.
engine, err := initEngine(args[0])
if engine.Type() == storage.MEM {
glog.Fatal("Cannot initialize a cockroach cluster using an in-memory storage device")
}
if err != nil {
glog.Fatal(err)
}
if engine.Type() == storage.MEM {
glog.Fatal("Cannot initialize a cockroach cluster using an in-memory storage device")
}
// Generate a new cluster UUID.
clusterID := uuid.New()
if _, err := BootstrapCluster(clusterID, engine); err != nil {
Expand Down
24 changes: 16 additions & 8 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ A node exports an HTTP API with the following endpoints:
Key-value REST: http://%s%s
Structured Schema REST: http://%s%s
`, *httpAddr, *httpAddr, kv.KVKeyPrefix, *httpAddr, structured.StructuredKeyPrefix),
Run: runStart,
Run: runStart,
Flag: *flag.CommandLine,
}

type server struct {
Expand All @@ -106,7 +107,14 @@ func runStart(cmd *commander.Command, args []string) {
if err != nil {
glog.Fatal(err)
}
err = s.start(nil /* init engines from -data_dirs */)
// init engines from -data_dirs
engines, err := initEngines()
if err != nil {
glog.Fatal(err)
}

err = s.start(engines)

s.stop()
if err != nil {
glog.Fatal(err)
Expand Down Expand Up @@ -134,7 +142,7 @@ func initEngines() ([]storage.Engine, error) {
func initEngine(spec string) (storage.Engine, error) {
// Error if regexp doesn't match.
matches := dataDirRE.FindStringSubmatch(spec)
if matches == nil || len(matches) != 3 {
if matches == nil || len(matches) != 5 {
return nil, util.Errorf("invalid engine specification %q", spec)
}

Expand All @@ -147,21 +155,21 @@ func initEngine(spec string) (storage.Engine, error) {
}
engine = storage.NewInMem(size)
} else {
// type, file = matches[3], matches[4]
Copy link
Member

Choose a reason for hiding this comment

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

We really do need a test for this.

var typ storage.DiskType
switch matches[2] {
switch matches[3] {
case "hdd":
typ = storage.HDD
case "ssd":
typ = storage.SSD
default:
return nil, util.Errorf("unhandled disk type %q", matches[1])
return nil, util.Errorf("unhandled disk type %q", matches[3])
}
engine, err = storage.NewRocksDB(typ, matches[2])
engine, err = storage.NewRocksDB(typ, matches[4])
if err != nil {
return nil, util.Errorf("unable to init rocksdb with data dir %q", matches[2])
return nil, util.Errorf("unable to init rocksdb with data dir %q", matches[4])
}
}

return engine, nil
}

Expand Down
17 changes: 13 additions & 4 deletions server/zone_cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package server
import (
"bytes"
"encoding/json"
"flag"
"fmt"
"io/ioutil"
"net/http"
Expand Down Expand Up @@ -61,7 +62,9 @@ Fetches and displays the zone configuration for <key-prefix>. The key
prefix should be escaped via URL query escaping if it contains
non-ascii bytes or spaces.
`,
Run: runGetZones}
Run: runGetZones,
Flag: *flag.CommandLine,
}

// runGetZones invokes the REST API with GET action and key prefix as path.
func runGetZones(cmd *commander.Command, args []string) {
Expand Down Expand Up @@ -93,7 +96,9 @@ the listing are filtered by key prefixes matching the regexp. The key
prefix should be escaped via URL query escaping if it contains
non-ascii bytes or spaces.
`,
Run: runLsZones}
Run: runLsZones,
Flag: *flag.CommandLine,
}

// runLsZones invokes the REST API with GET action and no path, which
// fetches a list of all zone configuration prefixes. The optional
Expand Down Expand Up @@ -150,7 +155,9 @@ command can affect only a single zone config with an exactly matching
prefix. The key prefix should be escaped via URL query escaping if it
contains non-ascii bytes or spaces.
`,
Run: runRmZone}
Run: runRmZone,
Flag: *flag.CommandLine,
}

// runRmZone invokes the REST API with DELETE action and key prefix as
// path.
Expand Down Expand Up @@ -204,7 +211,9 @@ Setting zone configs will guarantee that key ranges will be split
such that no key range straddles two zone config specifications.
This feature can be taken advantage of to pre-split ranges.
`,
Run: runSetZone}
Run: runSetZone,
Flag: *flag.CommandLine,
}

// runSetZone invokes the REST API with POST action and key prefix as
// path. The specified configuration file is read from disk and sent
Expand Down
3 changes: 2 additions & 1 deletion storage/rocksdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"flag"
"syscall"
"unsafe"

"github.com/golang/glog"
)

Expand Down Expand Up @@ -210,7 +211,7 @@ func (r *RocksDB) scan(start, end Key, max int64) ([]KeyValue, error) {
defer C.rocksdb_iter_destroy(it)

keyVals := []KeyValue{}
Copy link
Member

Choose a reason for hiding this comment

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

Is there an actual difference between these two statements in go? I confess I know very little about the cgo interface; Andy's worked on this part. I need to learn a bit more.

I noticed the continuous integration build is failing. Do the unittests pass for you?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see exactly what the problem is (sorry didn't read your PR note about scan on "").

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately that fix is wrong (as you pointed out - the tests), I have one that works now, it may not be ideal but I'm sure someone who knows their cgo will be able to improve. It will be on Phabricator.

C.rocksdb_iter_seek(it, (*C.char)(unsafe.Pointer(&start[0])), C.size_t(len(start)))
C.rocksdb_iter_seek(it, (*C.char)(unsafe.Pointer(&start)), C.size_t(len(start)))
for i := int64(1); C.rocksdb_iter_valid(it) == 1; C.rocksdb_iter_next(it) {
if max > 0 && i > max {
break
Expand Down