[perf] Use memcached in SQLTraceStore.
Also turns on pprof at port 6060.
Change-Id: I256b276569628dea4577490edc53901cfb9b700d
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/305898
Commit-Queue: Joe Gregorio <jcgregorio@google.com>
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
diff --git a/perf/configs/cdb-android-prod.json b/perf/configs/cdb-android-prod.json
index a5cf598..83ed65f 100644
--- a/perf/configs/cdb-android-prod.json
+++ b/perf/configs/cdb-android-prod.json
@@ -8,7 +8,15 @@
"instance": "production",
"table": "perf-android",
"shards": 8,
- "namespace": "perf-androidmaster"
+ "namespace": "perf-androidmaster",
+ "cache": {
+ "memcached_servers": [
+ "perf-memcached-0.perf-memcached:11211",
+ "perf-memcached-1.perf-memcached:11211",
+ "perf-memcached-2.perf-memcached:11211"
+ ],
+ "namespace": "and"
+ }
},
"ingestion_config": {
"source_config": {
diff --git a/perf/configs/cdb-android-x.json b/perf/configs/cdb-android-x.json
index cfeb6bb..671d51a 100644
--- a/perf/configs/cdb-android-x.json
+++ b/perf/configs/cdb-android-x.json
@@ -8,7 +8,15 @@
"instance": "production",
"table": "perf-android-x",
"shards": 8,
- "namespace": "perf-android-x"
+ "namespace": "perf-android-x",
+ "cache": {
+ "memcached_servers": [
+ "perf-memcached-0.perf-memcached:11211",
+ "perf-memcached-1.perf-memcached:11211",
+ "perf-memcached-2.perf-memcached:11211"
+ ],
+ "namespace": "and_x"
+ }
},
"ingestion_config": {
"source_config": {
diff --git a/perf/configs/cdb-ct-prod.json b/perf/configs/cdb-ct-prod.json
index 693c065..30c1693 100644
--- a/perf/configs/cdb-ct-prod.json
+++ b/perf/configs/cdb-ct-prod.json
@@ -8,7 +8,15 @@
"instance": "production",
"table": "perf-ct",
"shards": 8,
- "namespace": "perf-ct"
+ "namespace": "perf-ct",
+ "cache": {
+ "memcached_servers": [
+ "perf-memcached-0.perf-memcached:11211",
+ "perf-memcached-1.perf-memcached:11211",
+ "perf-memcached-2.perf-memcached:11211"
+ ],
+ "namespace": "ct"
+ }
},
"ingestion_config": {
"source_config": {
diff --git a/perf/configs/cdb-flutter.json b/perf/configs/cdb-flutter.json
index 1b4a347..31aed1f 100644
--- a/perf/configs/cdb-flutter.json
+++ b/perf/configs/cdb-flutter.json
@@ -8,7 +8,15 @@
"instance": "production",
"table": "perf-flutter",
"shards": 8,
- "namespace": "perf-flutter"
+ "namespace": "perf-flutter",
+ "cache": {
+ "memcached_servers": [
+ "perf-memcached-0.perf-memcached:11211",
+ "perf-memcached-1.perf-memcached:11211",
+ "perf-memcached-2.perf-memcached:11211"
+ ],
+ "namespace": "flut-e"
+ }
},
"ingestion_config": {
"source_config": {
diff --git a/perf/configs/cdb-nano.json b/perf/configs/cdb-nano.json
index 233e0c0..84d455c 100644
--- a/perf/configs/cdb-nano.json
+++ b/perf/configs/cdb-nano.json
@@ -8,7 +8,15 @@
"instance": "production",
"table": "perf-skia",
"shards": 8,
- "namespace": "perf"
+ "namespace": "perf",
+ "cache": {
+ "memcached_servers": [
+ "perf-memcached-0.perf-memcached:11211",
+ "perf-memcached-1.perf-memcached:11211",
+ "perf-memcached-2.perf-memcached:11211"
+ ],
+ "namespace": "skia"
+ }
},
"ingestion_config": {
"source_config": {
diff --git a/perf/configs/flutter-flutter.json b/perf/configs/flutter-flutter.json
index 9adbd17..7445fee 100644
--- a/perf/configs/flutter-flutter.json
+++ b/perf/configs/flutter-flutter.json
@@ -4,7 +4,15 @@
"datastore_type": "cockroachdb",
"connection_string": "postgresql://root@perf-cockroachdb-public:26257/flutter_flutter?sslmode=disable",
"tile_size": 4096,
- "shards": 8
+ "shards": 8,
+ "cache": {
+ "memcached_servers": [
+ "perf-memcached-0.perf-memcached:11211",
+ "perf-memcached-1.perf-memcached:11211",
+ "perf-memcached-2.perf-memcached:11211"
+ ],
+ "namespace": "flut-f"
+ }
},
"ingestion_config": {
"source_config": {
diff --git a/perf/configs/flutter.json b/perf/configs/flutter.json
deleted file mode 100644
index ed48a2a..0000000
--- a/perf/configs/flutter.json
+++ /dev/null
@@ -1,29 +0,0 @@
-{
- "URL": "https://flutter-engine-perf.skia.org/",
- "data_store_config": {
- "datastore_type": "gcp",
- "connection_string": "postgresql://root@perf-cockroachdb-public:26257/flutter_engine?sslmode=disable",
- "tile_size": 256,
- "project": "skia-public",
- "instance": "production",
- "table": "perf-flutter",
- "shards": 8,
- "namespace": "perf-flutter"
- },
- "ingestion_config": {
- "source_config": {
- "source_type": "gcs",
- "project": "skia-public",
- "topic": "perf-ingestion-flutter",
- "sources": ["gs://flutter-skia-perf/flutter-engine"]
- },
- "branches": [],
- "file_ingestion_pubsub_topic_name": ""
- },
- "git_repo_config": {
- "url": "https://github.com/flutter/engine",
- "dir": "/tmp/flutter-engine",
- "debounce_commit_url": false,
- "commit_url": "%s/commit/%s"
- }
-}
diff --git a/perf/configs/local.json b/perf/configs/local.json
index b07a60b..5f361db 100644
--- a/perf/configs/local.json
+++ b/perf/configs/local.json
@@ -8,13 +8,15 @@
"instance": "production",
"table": "perf-skia",
"shards": 8,
- "namespace": "perf"
+ "namespace": "perf",
+ "cache": {}
},
"ingestion_config": {
"source_config": {
"source_type": "gcs",
"project": "skia-public",
"topic": "perf-ingestion-skia-production",
+ "subscription": "perf-ingestion-skia-production-cdb-prod",
"sources": [
"gs://skia-perf/nano-json-v1",
"gs://skia-perf/task-duration",
diff --git a/perf/go/builders/builders.go b/perf/go/builders/builders.go
index ae2d1e6..f0e06c8 100644
--- a/perf/go/builders/builders.go
+++ b/perf/go/builders/builders.go
@@ -128,7 +128,7 @@
if err != nil {
return nil, skerr.Wrap(err)
}
- return sqltracestore.New(db, perfsql.CockroachDBDialect, instanceConfig.DataStoreConfig.TileSize)
+ return sqltracestore.New(db, perfsql.CockroachDBDialect, instanceConfig.DataStoreConfig)
}
return nil, skerr.Fmt("Unknown datastore type: %q", instanceConfig.DataStoreConfig.DataStoreType)
}
diff --git a/perf/go/config/config.go b/perf/go/config/config.go
index 05b9f6d..eabead4 100644
--- a/perf/go/config/config.go
+++ b/perf/go/config/config.go
@@ -36,6 +36,24 @@
CockroachDBDataStoreType DataStoreType = "cockroachdb"
)
+// Cache is the configuration for the LRU cache used by the storage client.
+type Cache struct {
+ // MemcachedServers is a list of memcached server names and addresses, e.g.
+ // ['memcached-0:11211', 'memcached-1:11211', 'memcached-2:11211']
+ //
+ // If this list is empty then the server will fall back to using an
+ // in-memory LRU cache per instance.
+ MemcachedServers []string `json:"memcached_servers"`
+
+ // Namespace is the string to add to each key to avoid conflicts with more
+ // than one application or application instance using the same memcached
+ // server.
+ Namespace string `json:"namespace"`
+
+ // Size is the size of the LRU cache to use if memcached isn't being used.
+ Size int `json:"size"`
+}
+
// DataStoreConfig is the configuration for how Perf stores data.
type DataStoreConfig struct {
// DataStoreType determines what type of datastore to build. This value will
@@ -79,6 +97,9 @@
// regressions, and shortcuts should use. This value is only used for 'gcp'
// datastore types.
Namespace string `json:"namespace"`
+
+ // Cache is the configuration for the LRU cache used by the storage client.
+ Cache Cache `json:"cache"`
}
// SourceType determines what type of file.Source to build from a SourceConfig.
diff --git a/perf/go/dfbuilder/dfbuilder_test.go b/perf/go/dfbuilder/dfbuilder_test.go
index e89cd95..625ad46 100644
--- a/perf/go/dfbuilder/dfbuilder_test.go
+++ b/perf/go/dfbuilder/dfbuilder_test.go
@@ -42,7 +42,7 @@
db, cleanup := sqltest.NewCockroachDBForTests(t, CockroachDatabaseName, sqltest.ApplyMigrations)
defer cleanup()
- store, err := sqltracestore.New(db, perfsql.CockroachDBDialect, 256)
+ store, err := sqltracestore.New(db, perfsql.CockroachDBDialect, cfg.DataStoreConfig)
require.NoError(t, err)
tileMap := buildTileMapOffsetToIndex([]types.CommitNumber{0, 1, 255, 256, 257}, store)
@@ -82,7 +82,7 @@
instanceConfig.DataStoreConfig.TileSize = 6
- store, err := sqltracestore.New(db, perfsql.CockroachDBDialect, instanceConfig.DataStoreConfig.TileSize)
+ store, err := sqltracestore.New(db, perfsql.CockroachDBDialect, instanceConfig.DataStoreConfig)
require.NoError(t, err)
builder := NewDataFrameBuilderFromTraceStore(g, store)
diff --git a/perf/go/perfserver/cmd/root.go b/perf/go/perfserver/cmd/root.go
index 7ff5e27..e8af189 100644
--- a/perf/go/perfserver/cmd/root.go
+++ b/perf/go/perfserver/cmd/root.go
@@ -2,9 +2,13 @@
import (
"fmt"
+ "net/http"
"os"
+ _ "net/http/pprof" // pprof
+
"github.com/spf13/cobra"
+ "go.skia.org/infra/go/sklog"
)
// rootCmd represents the base command when called without any subcommands
@@ -24,6 +28,9 @@
// Execute adds all child commands to the root command and sets flags appropriately.
// This is called by main.main(). It only needs to happen once to the rootCmd.
func Execute() {
+ go func() {
+ sklog.Error(http.ListenAndServe(":6060", nil))
+ }()
if err := initSubCommands(); err != nil {
fmt.Println(err)
os.Exit(1)
diff --git a/perf/go/regression/dfiter_test.go b/perf/go/regression/dfiter_test.go
index 15c3efc..2fa2e9a 100644
--- a/perf/go/regression/dfiter_test.go
+++ b/perf/go/regression/dfiter_test.go
@@ -11,6 +11,7 @@
"go.skia.org/infra/go/query"
"go.skia.org/infra/go/testutils/unittest"
"go.skia.org/infra/perf/go/alerts"
+ "go.skia.org/infra/perf/go/config"
"go.skia.org/infra/perf/go/dataframe"
"go.skia.org/infra/perf/go/dfbuilder"
perfgit "go.skia.org/infra/perf/go/git"
@@ -47,7 +48,15 @@
func newForTest(t *testing.T) (context.Context, dataframe.DataFrameBuilder, *perfgit.Git, cleanupFunc) {
db, dbCleanup := sqltest.NewCockroachDBForTests(t, CockroachDatabaseName, sqltest.ApplyMigrations)
- store, err := sqltracestore.New(db, perfsql.CockroachDBDialect, testTileSize)
+ cfg := config.DataStoreConfig{
+ TileSize: testTileSize,
+ Project: "test",
+ Instance: "test",
+ Table: "test",
+ Shards: 8,
+ }
+
+ store, err := sqltracestore.New(db, perfsql.CockroachDBDialect, cfg)
require.NoError(t, err)
// Add some points to the first and second tile.
diff --git a/perf/go/tracestore/sqltracestore/sqltracestore.go b/perf/go/tracestore/sqltracestore/sqltracestore.go
index 7fb2645..a0e59d8 100644
--- a/perf/go/tracestore/sqltracestore/sqltracestore.go
+++ b/perf/go/tracestore/sqltracestore/sqltracestore.go
@@ -144,17 +144,17 @@
"go.skia.org/infra/go/vec32"
"go.skia.org/infra/perf/go/cache"
"go.skia.org/infra/perf/go/cache/local"
+ "go.skia.org/infra/perf/go/cache/memcached"
+ "go.skia.org/infra/perf/go/config"
perfsql "go.skia.org/infra/perf/go/sql"
"go.skia.org/infra/perf/go/tracestore"
"go.skia.org/infra/perf/go/tracestore/sqltracestore/engine"
"go.skia.org/infra/perf/go/types"
)
-// cacheSize is the size of the LRU cache.
-//
-// TODO(jcgregorio) Move to config.InstanceConfig since this should be tweaked
-// per instance.
-const cacheSize = 20 * 1000 * 1000
+// defaultCacheSize is the size of the in-memory LRU cache if no size was
+// specified in the config file.
+const defaultCacheSize = 20 * 1000 * 1000
// Ingest instances have around 8 cores and many ingested files have ~10K
// values, so pick a batch size that allows roughly one request per core.
@@ -333,9 +333,9 @@
// metrics
writeTracesMetric metrics2.Float64SummaryMetric
+ writeTracesMetricSQL metrics2.Float64SummaryMetric
updateTraceValuesMetric metrics2.Float64SummaryMetric
buildTracesContextMetric metrics2.Float64SummaryMetric
- writeTraceIDAndPostingsMetric metrics2.Float64SummaryMetric
writeTraceIDAndPostingsCacheHitMetric metrics2.Counter
writeTraceIDAndPostingsCacheMissMetric metrics2.Counter
writeTraceIDAndPostingsPostingsCacheHitMetric metrics2.Counter
@@ -346,7 +346,7 @@
//
// We presume all migrations have been run against db before this function is
// called.
-func New(db *sql.DB, dialect perfsql.Dialect, tileSize int32) (*SQLTraceStore, error) {
+func New(db *sql.DB, dialect perfsql.Dialect, datastoreConfig config.DataStoreConfig) (*SQLTraceStore, error) {
preparedStatements := map[statement]*sql.Stmt{}
for key, statement := range statementsByDialect[dialect] {
prepared, err := db.Prepare(statement)
@@ -365,9 +365,28 @@
unpreparedStatements[key] = t
}
- cache, err := local.New(cacheSize)
- if err != nil {
- return nil, skerr.Wrap(err)
+ var memoryCacheSize = datastoreConfig.Cache.Size
+ if memoryCacheSize == 0 {
+ memoryCacheSize = defaultCacheSize
+ }
+
+ var err error
+ var cache cache.Cache
+ if len(datastoreConfig.Cache.MemcachedServers) > 0 && datastoreConfig.Cache.Namespace != "" {
+ cache, err = memcached.New(datastoreConfig.Cache.MemcachedServers, datastoreConfig.Cache.Namespace)
+ if err != nil {
+ // Fall back to in-memory cache.
+ cache, err = local.New(memoryCacheSize)
+ if err != nil {
+ return nil, skerr.Wrap(err)
+ }
+
+ }
+ } else {
+ cache, err = local.New(memoryCacheSize)
+ if err != nil {
+ return nil, skerr.Wrap(err)
+ }
}
return &SQLTraceStore{
@@ -375,11 +394,11 @@
preparedStatements: preparedStatements,
unpreparedStatements: unpreparedStatements,
cache: cache,
- tileSize: tileSize,
+ tileSize: datastoreConfig.TileSize,
writeTracesMetric: metrics2.GetFloat64SummaryMetric("perfserver_sqltracestore_writeTraces"),
+ writeTracesMetricSQL: metrics2.GetFloat64SummaryMetric("perfserver_sqltracestore_writeTracesSQL"),
updateTraceValuesMetric: metrics2.GetFloat64SummaryMetric("perfserver_sqltracestore_updateTraceValues"),
buildTracesContextMetric: metrics2.GetFloat64SummaryMetric("perfserver_sqltracestore_buildTracesContext"),
- writeTraceIDAndPostingsMetric: metrics2.GetFloat64SummaryMetric("perfserver_sqltracestore_writeTraceIDAndPostings"),
writeTraceIDAndPostingsCacheHitMetric: metrics2.GetCounter("perfserver_sqltracestore_writeTraceIDAndPostings_cache_hit"),
writeTraceIDAndPostingsCacheMissMetric: metrics2.GetCounter("perfserver_sqltracestore_writeTraceIDAndPostings_cache_miss"),
writeTraceIDAndPostingsPostingsCacheHitMetric: metrics2.GetCounter("perfserver_sqltracestore_writeTraceIDAndPostings_postings_cache_hit"),
@@ -814,7 +833,6 @@
// returns the traceIDFromSQL of that trace name. This operation will happen
// repeatedly as data is ingested so we cache the results in the LRU cache.
func (s *SQLTraceStore) writeTraceIDAndPostings(traceNameAsParams paramtools.Params, tileNumber types.TileNumber) (traceIDFromSQL, error) {
- defer timer.NewWithSummary("perfserver_sqltracestore_writeTraceIDAndPostings", s.writeTraceIDAndPostingsMetric).Stop()
traceID := badTraceIDFromSQL
traceName, err := query.MakeKey(traceNameAsParams)
@@ -895,6 +913,7 @@
}
t.Stop()
+ defer timer.NewWithSummary("perfserver_sqltracestore_writeTraces_sql", s.writeTracesMetricSQL).Stop()
err = util.ChunkIterParallel(context.TODO(), len(templateContext), traceValuesInsertBatchSize, func(ctx context.Context, startIdx int, endIdx int) error {
if err := s.updateTraceValues(templateContext[startIdx:endIdx]); err != nil {
return skerr.Wrapf(err, "failed inserting subSlice: [%d:%d]", startIdx, endIdx)
diff --git a/perf/go/tracestore/sqltracestore/sqltracestore_test.go b/perf/go/tracestore/sqltracestore/sqltracestore_test.go
index a1d6040..9d404bc 100644
--- a/perf/go/tracestore/sqltracestore/sqltracestore_test.go
+++ b/perf/go/tracestore/sqltracestore/sqltracestore_test.go
@@ -12,6 +12,7 @@
"go.skia.org/infra/go/query"
"go.skia.org/infra/go/testutils/unittest"
"go.skia.org/infra/go/vec32"
+ "go.skia.org/infra/perf/go/config"
perfsql "go.skia.org/infra/perf/go/sql"
"go.skia.org/infra/perf/go/sql/sqltest"
"go.skia.org/infra/perf/go/types"
@@ -28,6 +29,14 @@
func TestCockroachDB(t *testing.T) {
unittest.LargeTest(t)
+ cfg := config.DataStoreConfig{
+ TileSize: testTileSize,
+ Project: "test",
+ Instance: "test",
+ Table: "test",
+ Shards: 8,
+ }
+
for name, subTest := range subTests {
t.Run(name, func(t *testing.T) {
db, cleanup := sqltest.NewCockroachDBForTests(t, "tracestore", sqltest.ApplyMigrations)
@@ -35,7 +44,7 @@
// easier to understand.
defer cleanup()
- store, err := New(db, perfsql.CockroachDBDialect, testTileSize)
+ store, err := New(db, perfsql.CockroachDBDialect, cfg)
require.NoError(t, err)
subTest(t, store)