Skip to content

Commit

Permalink
Move storage providers (#1919)
Browse files Browse the repository at this point in the history
This change moves the individual storage providers to the corresponding implementation packages. This makes the server package agnostic of storage implementations, and makes it the responsibility of main.go files to import them.
  • Loading branch information
pav-kv authored Oct 23, 2019
1 parent 1d870d5 commit 8162a59
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 53 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ SetAndVerifyMapLeaves method was deleted.
The StorageProvider type and helpers have been moved from the server package to
storage. Aliases for the old types/functions are created for backward
compatibility, but the new code should not use them as we will remove them with
the next major version bump.
the next major version bump. The individual storage providers have been moved to
the corresponding packages, and are now required to be imported explicitly by
the main file in order to be registered. We are including only MySQL and
cloudspanner providers by default, since these are the ones that we support.

## v1.3.2 - Module fixes

Expand Down
3 changes: 2 additions & 1 deletion server/mysql_quota_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/golang/glog"
"github.com/google/trillian/quota"
"github.com/google/trillian/quota/mysqlqm"
"github.com/google/trillian/storage/mysql"
)

// QuotaMySQL represents the MySQL quota implementation.
Expand All @@ -37,7 +38,7 @@ func init() {
}

func newMySQLQuotaManager() (quota.Manager, error) {
db, err := getMySQLDatabase()
db, err := mysql.GetDatabase()
if err != nil {
return nil, err
}
Expand Down
4 changes: 4 additions & 0 deletions server/trillian_log_server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ import (
_ "github.com/google/trillian/crypto/keys/pem/proto"
_ "github.com/google/trillian/crypto/keys/pkcs11/proto"

// Register supported storage providers.
_ "github.com/google/trillian/storage/cloudspanner"
_ "github.com/google/trillian/storage/mysql"

// Load hashers
_ "github.com/google/trillian/merkle/rfc6962"
)
Expand Down
4 changes: 4 additions & 0 deletions server/trillian_log_signer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ import (
_ "github.com/google/trillian/crypto/keys/pem/proto"
_ "github.com/google/trillian/crypto/keys/pkcs11/proto"

// Register supported storage providers.
_ "github.com/google/trillian/storage/cloudspanner"
_ "github.com/google/trillian/storage/mysql"

// Load hashers
_ "github.com/google/trillian/merkle/rfc6962"
)
Expand Down
4 changes: 4 additions & 0 deletions server/trillian_map_server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ import (
_ "github.com/google/trillian/crypto/keys/pem/proto"
_ "github.com/google/trillian/crypto/keys/pkcs11/proto"

// Register supported storage providers.
_ "github.com/google/trillian/storage/cloudspanner"
_ "github.com/google/trillian/storage/mysql"

// Load hashers
_ "github.com/google/trillian/merkle/coniks"
_ "github.com/google/trillian/merkle/maphasher"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package server
package cloudspanner

import (
"bytes"
Expand All @@ -28,7 +28,6 @@ import (
"github.com/golang/glog"
"github.com/google/trillian/monitoring"
"github.com/google/trillian/storage"
"github.com/google/trillian/storage/cloudspanner"
)

var (
Expand All @@ -50,7 +49,7 @@ var (
)

func init() {
if err := RegisterStorageProvider("cloud_spanner", newCloudSpannerStorageProvider); err != nil {
if err := storage.RegisterProvider("cloud_spanner", newCloudSpannerStorageProvider); err != nil {
panic(err)
}
}
Expand Down Expand Up @@ -88,7 +87,7 @@ func configFromFlags() spanner.ClientConfig {
return r
}

func newCloudSpannerStorageProvider(_ monitoring.MetricFactory) (StorageProvider, error) {
func newCloudSpannerStorageProvider(_ monitoring.MetricFactory) (storage.Provider, error) {
csMu.Lock()
defer csMu.Unlock()

Expand All @@ -109,7 +108,7 @@ func newCloudSpannerStorageProvider(_ monitoring.MetricFactory) (StorageProvider
// LogStorage builds and returns a new storage.LogStorage using CloudSpanner.
func (s *cloudSpannerProvider) LogStorage() storage.LogStorage {
warn()
opts := cloudspanner.LogStorageOptions{}
opts := LogStorageOptions{}
frac := *csDequeueAcrossMerkleBucketsFraction
if frac > 1.0 {
frac = 1.0
Expand All @@ -121,23 +120,23 @@ func (s *cloudSpannerProvider) LogStorage() storage.LogStorage {
if *csReadOnlyStaleness > 0 {
opts.ReadOnlyStaleness = *csReadOnlyStaleness
}
return cloudspanner.NewLogStorageWithOpts(s.client, opts)
return NewLogStorageWithOpts(s.client, opts)
}

// MapStorage builds and returns a new storage.MapStorage using CloudSpanner.
func (s *cloudSpannerProvider) MapStorage() storage.MapStorage {
warn()
opts := cloudspanner.MapStorageOptions{}
opts := MapStorageOptions{}
if *csReadOnlyStaleness > 0 {
opts.ReadOnlyStaleness = *csReadOnlyStaleness
}
return cloudspanner.NewMapStorageWithOpts(s.client, opts)
return NewMapStorageWithOpts(s.client, opts)
}

// AdminStorage builds and returns a new storage.AdminStorage using CloudSpanner.
func (s *cloudSpannerProvider) AdminStorage() storage.AdminStorage {
warn()
return cloudspanner.NewAdminStorage(s.client)
return NewAdminStorage(s.client)
}

// Close shuts down this provider. Calls to the other methods will fail
Expand Down
15 changes: 7 additions & 8 deletions server/memory_storage_provider.go → storage/memory/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,43 +12,42 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package server
package memory

import (
"github.com/golang/glog"
"github.com/google/trillian/monitoring"
"github.com/google/trillian/storage"
"github.com/google/trillian/storage/memory"
)

func init() {
if err := RegisterStorageProvider("memory", newMemoryStorageProvider); err != nil {
if err := storage.RegisterProvider("memory", newMemoryStorageProvider); err != nil {
glog.Fatalf("Failed to register storage provider memory: %v", err)
}
}

type memProvider struct {
mf monitoring.MetricFactory
ts *memory.TreeStorage
ts *TreeStorage
}

func newMemoryStorageProvider(mf monitoring.MetricFactory) (StorageProvider, error) {
func newMemoryStorageProvider(mf monitoring.MetricFactory) (storage.Provider, error) {
return &memProvider{
mf: mf,
ts: memory.NewTreeStorage(),
ts: NewTreeStorage(),
}, nil
}

func (s *memProvider) LogStorage() storage.LogStorage {
return memory.NewLogStorage(s.ts, s.mf)
return NewLogStorage(s.ts, s.mf)
}

func (s *memProvider) MapStorage() storage.MapStorage {
return nil
}

func (s *memProvider) AdminStorage() storage.AdminStorage {
return memory.NewAdminStorage(s.ts)
return NewAdminStorage(s.ts)
}

func (s *memProvider) Close() error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package server
package memory

import (
"testing"

"github.com/google/trillian/storage"
)

func TestMemoryStorageProvider(t *testing.T) {
sp, err := NewStorageProvider("memory", nil)
sp, err := storage.NewProvider("memory", nil)
if err != nil {
t.Fatalf("Got an unexpected error: %v", err)
}
Expand All @@ -29,7 +31,7 @@ func TestMemoryStorageProvider(t *testing.T) {
}

func TestMemoryStorageProviderLogStorage(t *testing.T) {
sp, err := NewStorageProvider("memory", nil)
sp, err := storage.NewProvider("memory", nil)
if err != nil {
t.Fatalf("Got an unexpected error: %v", err)
}
Expand All @@ -41,7 +43,7 @@ func TestMemoryStorageProviderLogStorage(t *testing.T) {
}

func TestMemoryStorageProviderMapStorage(t *testing.T) {
sp, err := NewStorageProvider("memory", nil)
sp, err := storage.NewProvider("memory", nil)
if err != nil {
t.Fatalf("Got an unexpected error: %v", err)
}
Expand All @@ -53,7 +55,7 @@ func TestMemoryStorageProviderMapStorage(t *testing.T) {
}

func TestMemoryStorageProviderAdminStorage(t *testing.T) {
sp, err := NewStorageProvider("memory", nil)
sp, err := storage.NewProvider("memory", nil)
if err != nil {
t.Fatalf("Got an unexpected error: %v", err)
}
Expand All @@ -65,7 +67,7 @@ func TestMemoryStorageProviderAdminStorage(t *testing.T) {
}

func TestMemoryStorageProviderClose(t *testing.T) {
sp, err := NewStorageProvider("memory", nil)
sp, err := storage.NewProvider("memory", nil)
if err != nil {
t.Fatalf("Got an unexpected error: %v", err)
}
Expand Down
32 changes: 17 additions & 15 deletions server/mysql_storage_provider.go → storage/mysql/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package server
package mysql

import (
"database/sql"
Expand All @@ -22,7 +22,6 @@ import (
"github.com/golang/glog"
"github.com/google/trillian/monitoring"
"github.com/google/trillian/storage"
"github.com/google/trillian/storage/mysql"

// Load MySQL driver
_ "github.com/go-sql-driver/mysql"
Expand All @@ -39,8 +38,18 @@ var (
mysqlStorageInstance *mysqlProvider
)

// GetDatabase returns an instance of MySQL database, or creates one.
//
// TODO(pavelkalinnikov): Make the dependency of MySQL quota provider from
// MySQL storage provider explicit.
func GetDatabase() (*sql.DB, error) {
mysqlMu.Lock()
defer mysqlMu.Unlock()
return getMySQLDatabaseLocked()
}

func init() {
if err := RegisterStorageProvider("mysql", newMySQLStorageProvider); err != nil {
if err := storage.RegisterProvider("mysql", newMySQLStorageProvider); err != nil {
glog.Fatalf("Failed to register storage provider mysql: %v", err)
}
}
Expand All @@ -50,7 +59,7 @@ type mysqlProvider struct {
mf monitoring.MetricFactory
}

func newMySQLStorageProvider(mf monitoring.MetricFactory) (StorageProvider, error) {
func newMySQLStorageProvider(mf monitoring.MetricFactory) (storage.Provider, error) {
mysqlMu.Lock()
defer mysqlMu.Unlock()
if mysqlStorageInstance == nil {
Expand All @@ -66,20 +75,13 @@ func newMySQLStorageProvider(mf monitoring.MetricFactory) (StorageProvider, erro
return mysqlStorageInstance, nil
}

// getMySQLDatabase returns an instance of MySQL database, or creates one.
func getMySQLDatabase() (*sql.DB, error) {
mysqlMu.Lock()
defer mysqlMu.Unlock()
return getMySQLDatabaseLocked()
}

// getMySQLDatabaseLocked returns an instance of MySQL database, or creates
// one. Requires mysqlMu to be locked.
func getMySQLDatabaseLocked() (*sql.DB, error) {
if mysqlDB != nil || mysqlErr != nil {
return mysqlDB, mysqlErr
}
db, err := mysql.OpenDB(*mySQLURI)
db, err := OpenDB(*mySQLURI)
if err != nil {
mysqlErr = err
return nil, err
Expand All @@ -95,15 +97,15 @@ func getMySQLDatabaseLocked() (*sql.DB, error) {
}

func (s *mysqlProvider) LogStorage() storage.LogStorage {
return mysql.NewLogStorage(s.db, s.mf)
return NewLogStorage(s.db, s.mf)
}

func (s *mysqlProvider) MapStorage() storage.MapStorage {
return mysql.NewMapStorage(s.db)
return NewMapStorage(s.db)
}

func (s *mysqlProvider) AdminStorage() storage.AdminStorage {
return mysql.NewAdminStorage(s.db)
return NewAdminStorage(s.db)
}

func (s *mysqlProvider) Close() error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package server
package mysql

import (
"flag"
"testing"

"github.com/google/trillian/storage"
"github.com/google/trillian/testonly/flagsaver"
)

Expand All @@ -28,18 +29,18 @@ func TestMySQLStorageProviderErrorPersistence(t *testing.T) {
}

// First call: This should fail due to the Database URL being garbage.
_, err1 := NewStorageProvider("mysql", nil)
_, err1 := storage.NewProvider("mysql", nil)
if err1 == nil {
t.Fatalf("Expected 'server.NewStorageProvider' to fail")
t.Fatalf("Expected 'storage.NewProvider' to fail")
}

// Second call: This should fail with the same error.
_, err2 := NewStorageProvider("mysql", nil)
_, err2 := storage.NewProvider("mysql", nil)
if err2 == nil {
t.Fatalf("Expected second call to 'server.NewStorageProvider' to fail")
t.Fatalf("Expected second call to 'storage.NewProvider' to fail")
}

if err2 != err1 {
t.Fatalf("Expected second call to 'server.NewStorageProvider' to fail with %q, instead got: %q", err1, err2)
t.Fatalf("Expected second call to 'storage.NewProvider' to fail with %q, instead got: %q", err1, err2)
}
}
Loading

0 comments on commit 8162a59

Please sign in to comment.