Skip to content

Commit

Permalink
cli: set up logging for start-sql
Browse files Browse the repository at this point in the history
The `--log-dir` flag is now supported.

Release justification: low-risk and with high impact on free tier
observability.
Release note: None
  • Loading branch information
tbg committed Sep 4, 2020
1 parent d0163ed commit 638080a
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 6 deletions.
1 change: 1 addition & 0 deletions pkg/cli/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,7 @@ func init() {
varFlag(f, addrSetter{&serverHTTPAddr, &serverHTTPPort}, cliflags.ListenHTTPAddr)

stringSliceFlag(f, &serverCfg.SQLConfig.TenantKVAddrs, cliflags.KVAddrs)
varFlag(f, &startCtx.logDir, cliflags.LogDir)
}
}

Expand Down
14 changes: 12 additions & 2 deletions pkg/cli/mt_start_sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/stop"
"github.com/cockroachdb/errors"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -61,7 +60,18 @@ well unless it can be verified using a trusted root certificate store. That is,
func runStartSQL(cmd *cobra.Command, args []string) error {
ctx := context.Background()
const clusterName = ""
stopper := stop.NewStopper()

// Remove the default store, which avoids using it to set up logging.
// Instead, we'll default to logging to stderr unless --log-dir is
// specified. This makes sense since the standalone SQL server is
// at the time of writing stateless and may not be provisioned with
// suitable storage.
serverCfg.Stores.Specs = nil

stopper, err := setupAndInitializeLoggingAndProfiling(ctx, cmd)
if err != nil {
return err
}
defer stopper.Stop(ctx)

st := serverCfg.BaseConfig.Settings
Expand Down
3 changes: 2 additions & 1 deletion pkg/cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,7 @@ func logOutputDirectory() string {
// setupAndInitializeLoggingAndProfiling does what it says on the label.
// Prior to this however it determines suitable defaults for the
// logging output directory and the verbosity level of stderr logging.
// We only do this for the "start" command which is why this work
// We only do this for the "start" and "start-sql" commands which is why this work
// occurs here and not in an OnInitialize function.
func setupAndInitializeLoggingAndProfiling(
ctx context.Context, cmd *cobra.Command,
Expand Down Expand Up @@ -1031,6 +1031,7 @@ func setupAndInitializeLoggingAndProfiling(
}
newDir = filepath.Join(spec.Path, "logs")
}

if err := startCtx.logDir.Set(newDir); err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const (
defaultScanMinIdleTime = 10 * time.Millisecond
defaultScanMaxIdleTime = 1 * time.Second

defaultStorePath = "cockroach-data"
DefaultStorePath = "cockroach-data"
// TempDirPrefix is the filename prefix of any temporary subdirectory
// created.
TempDirPrefix = "cockroach-temp"
Expand Down Expand Up @@ -384,7 +384,7 @@ func SetOpenFileLimitForOneStore() (uint64, error) {

// MakeConfig returns a Config for the system tenant with default values.
func MakeConfig(ctx context.Context, st *cluster.Settings) Config {
storeSpec, err := base.NewStoreSpec(defaultStorePath)
storeSpec, err := base.NewStoreSpec(DefaultStorePath)
if err != nil {
panic(err)
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/server/testserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,9 @@ func makeSQLServerArgs(
const sqlInstanceID = base.SQLInstanceID(10001)
idContainer := base.NewSQLIDContainer(sqlInstanceID, &c, false /* exposed */)

runtime := status.NewRuntimeStatSampler(context.Background(), clock)
registry.AddMetricStruct(runtime)

// We don't need this for anything except some services that want a gRPC
// server to register against (but they'll never get RPCs at the time of
// writing): the blob service and DistSQL.
Expand Down Expand Up @@ -573,7 +576,7 @@ func makeSQLServerArgs(
BaseConfig: &baseCfg,
stopper: stopper,
clock: clock,
runtime: status.NewRuntimeStatSampler(context.Background(), clock),
runtime: runtime,
rpcContext: rpcContext,
nodeDescs: tenantConnect,
systemConfigProvider: tenantConnect,
Expand Down

0 comments on commit 638080a

Please sign in to comment.