Add HasDirective to dax.Node struct to force Directive on restart (#2335)

For on-prem serverless, if we restart the process containing the
controller and computer(s), when they come back up, the controller
doesn't know that the computers have been restarted, so it doesn't send
them a directive. This change forces the controller to send a directive
upon startup by a computer.
This commit is contained in:
Travis Turner 2023-03-21 13:22:16 -05:00 committed by GitHub
parent 2cf972b5d1
commit 2f7ae30784
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 267 additions and 117 deletions

View file

@ -92,6 +92,12 @@ func (db *DB) InitializeBuckets(buckets ...Bucket) (err error) {
})
}
// Start is here to implement the Transactor interface, but we don't really need
// it in the BoltDB implementation.
func (db *DB) Start() (err error) {
return nil
}
// Open opens the database connection.
func (db *DB) Open() (err error) {
path, err := db.path()

View file

@ -74,6 +74,7 @@ func (c *computerService) Start() error {
dax.RoleTypeCompute,
dax.RoleTypeTranslate,
},
HasDirective: false,
}
if err := c.computer.Registrar.RegisterNode(context.TODO(), node); err != nil {

View file

@ -24,9 +24,13 @@ func run(m *testing.M) int {
// We connect to a randomized database, create it, and run migrations. Then we drop it when tests are done.
conf := sqldb.GetTestConfigRandomDB("balancer_test")
var err error
SQLTransactor, err = sqldb.Connect(conf, logger.StderrLogger)
SQLTransactor, err = sqldb.NewTransactor(conf, logger.StderrLogger)
if err != nil {
fmt.Printf("couldn't make connection to database: %v", err)
fmt.Printf("couldn't set up transactor: %v", err)
return -1
}
if err := SQLTransactor.Start(); err != nil {
fmt.Printf("couldn't start transactor: %v", err)
return -1
}

View file

@ -618,9 +618,10 @@ func (c *Client) RegisterNode(ctx context.Context, node *dax.Node) error {
url := fmt.Sprintf("%s/register-node", c.address.WithScheme(defaultScheme))
c.logger.Debugf("RegisterNode: %s, url: %s", node.Address, url)
req := &controllerhttp.RegisterNodeRequest{
Address: node.Address,
RoleTypes: node.RoleTypes,
req := &dax.Node{
Address: node.Address,
RoleTypes: node.RoleTypes,
HasDirective: node.HasDirective,
}
// Encode the request.
@ -648,9 +649,10 @@ func (c *Client) CheckInNode(ctx context.Context, node *dax.Node) error {
url := fmt.Sprintf("%s/check-in-node", c.address.WithScheme(defaultScheme))
c.logger.Debugf("CheckInNode url: %s", url)
req := &controllerhttp.CheckInNodeRequest{
Address: node.Address,
RoleTypes: node.RoleTypes,
req := &dax.Node{
Address: node.Address,
RoleTypes: node.RoleTypes,
HasDirective: node.HasDirective,
}
// Encode the request.

View file

@ -77,8 +77,7 @@ func New(cfg Config) *Controller {
snappingTurtleTimeout: cfg.SnappingTurtleTimeout,
snapControl: make(chan struct{}),
stopping: make(chan struct{}),
logger: logr,
logger: logr,
}
// Poller.
@ -102,6 +101,12 @@ func New(cfg Config) *Controller {
// Start starts long running subroutines.
func (c *Controller) Start() error {
// Set up the stopping channel here in case the controller restarts.
c.stopping = make(chan struct{})
if err := c.Transactor.Start(); err != nil {
return errors.Wrap(err, "starting transactor")
}
c.backgroundGroup.Go(c.poller.Run) // TODO: this could just use c.stopping as well?
@ -172,6 +177,11 @@ func (c *Controller) RegisterNodes(ctx context.Context, nodes ...*dax.Node) erro
for _, n := range nodes {
// If the node already exists, skip it.
if node, _ := c.Balancer.ReadNode(tx, n.Address); node != nil {
// If the node already exists, but it has indicated that it doesn't
// have a directive, then send it one.
if !n.HasDirective {
workerSet.Add(n.Address)
}
continue
}
@ -260,7 +270,10 @@ func (c *Controller) RegisterNode(ctx context.Context, n *dax.Node) error {
}
defer tx.Rollback()
if node, _ := c.Balancer.ReadNode(tx, n.Address); node != nil {
// If the node is telling us that it doesn't have a directive, let it
// continue because we need to send it one even though we already think we
// know about it.
if node, _ := c.Balancer.ReadNode(tx, n.Address); node != nil && n.HasDirective {
return nil
}
@ -291,7 +304,11 @@ func (c *Controller) CheckInNode(ctx context.Context, n *dax.Node) error {
// Directive; then we could check that the compute node is actually doing
// what we expect it to be doing. But for now, we're just checking that we
// know about the compute node at all.
if node, _ := c.Balancer.ReadNode(tx, n.Address); node != nil {
//
// However, if the node is telling us that it doesn't have a directive, let
// it continue because we need to send it one even though we already think
// we know about it.
if node, _ := c.Balancer.ReadNode(tx, n.Address); node != nil && n.HasDirective {
return nil
}

View file

@ -610,6 +610,7 @@ func TestController(t *testing.T) {
RoleTypes: []dax.RoleType{
dax.RoleTypeTranslate,
},
HasDirective: true,
}
assert.NoError(t, con.RegisterNodes(ctx, node0))

View file

@ -603,17 +603,12 @@ func (s *server) postRegisterNode(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
req := RegisterNodeRequest{}
if err := json.NewDecoder(body).Decode(&req); err != nil {
node := &dax.Node{}
if err := json.NewDecoder(body).Decode(node); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
node := &dax.Node{
Address: req.Address,
RoleTypes: req.RoleTypes,
}
if err := s.controller.RegisterNode(ctx, node); err != nil {
http.Error(w, errors.MarshalJSON(err), http.StatusBadRequest)
return
@ -622,15 +617,6 @@ func (s *server) postRegisterNode(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}
type RegisterNodeRequest struct {
Address dax.Address `json:"address"`
// RoleTypes allows a registering node to specify which role type(s) it is
// capable of filling. The controller will not assign a role to this node
// with a type not included in RoleTypes.
RoleTypes []dax.RoleType `json:"role-types"`
}
// POST /register-nodes
func (s *server) postRegisterNodes(w http.ResponseWriter, r *http.Request) {
body := r.Body
@ -688,17 +674,12 @@ func (s *server) postCheckInNode(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
req := CheckInNodeRequest{}
if err := json.NewDecoder(body).Decode(&req); err != nil {
node := &dax.Node{}
if err := json.NewDecoder(body).Decode(node); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
node := &dax.Node{
Address: req.Address,
RoleTypes: req.RoleTypes,
}
if err := s.controller.CheckInNode(ctx, node); err != nil {
http.Error(w, errors.MarshalJSON(err), http.StatusBadRequest)
return
@ -707,15 +688,6 @@ func (s *server) postCheckInNode(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}
type CheckInNodeRequest struct {
Address dax.Address `json:"address"`
// RoleTypes allows a registering node to specify which role type(s) it is
// capable of filling. The controller will not assign a role to this node
// with a type not included in RoleTypes.
RoleTypes []dax.RoleType `json:"role-types"`
}
// POST /compute-nodes
func (s *server) postComputeNodes(w http.ResponseWriter, r *http.Request) {
body := r.Body

View file

@ -33,7 +33,6 @@ func New(cfg Config) *Poller {
nodeService: dax.NewNopNodeService(),
nodePoller: NewNopNodePoller(),
pollInterval: time.Second,
stopping: make(chan struct{}),
logger: logger.NopLogger,
}
@ -73,6 +72,10 @@ func (p *Poller) Addresses() []dax.Address {
// Run starts the polling goroutine.
func (p *Poller) Run() error {
// Set up the stopping channel here in case the controller restarts and runs
// the Poller again.
p.stopping = make(chan struct{})
p.run()
return nil
}

View file

@ -34,10 +34,12 @@ var (
func TestSQLSchemar(t *testing.T) {
conf := sqldb.GetTestConfigRandomDB("sql_schemar")
trans, err := sqldb.Connect(conf, logger.StderrLogger)
trans, err := sqldb.NewTransactor(conf, logger.StderrLogger)
require.NoError(t, err, "connecting")
defer sqldb.DropDatabase(trans)
require.NoError(t, trans.Start())
tx, err := trans.BeginTx(context.Background(), true)
require.NoError(t, err, "getting transaction")

View file

@ -74,9 +74,9 @@ func New(uri *fbnet.URI, cfg controller.Config) *controllerService {
controller.Balancer = sqldb.NewBalancer(logr)
controller.DirectiveVersion = sqldb.NewDirectiveVersion(logr)
transactor, err := sqldb.Connect(cfg.SQLDB, logr)
transactor, err := sqldb.NewTransactor(cfg.SQLDB, logr)
if err != nil {
logr.Printf("Connecting to database: %v", err)
logr.Printf("setting up new transactor: %v", err)
os.Exit(1)
}
controller.Transactor = transactor

View file

@ -1,58 +0,0 @@
package sqldb
import (
"github.com/featurebasedb/featurebase/v3/dax"
"github.com/featurebasedb/featurebase/v3/dax/controller"
"github.com/featurebasedb/featurebase/v3/errors"
"github.com/featurebasedb/featurebase/v3/logger"
"github.com/gobuffalo/pop/v6"
)
func Connect(cfg *controller.SQLDBConfig, log logger.Logger) (Transactor, error) {
conn, err := pop.NewConnection(&pop.ConnectionDetails{
Dialect: cfg.Dialect,
Database: cfg.Database,
Host: cfg.Host,
Port: cfg.Port,
User: cfg.User,
Password: cfg.Password,
URL: cfg.URL,
Pool: cfg.Pool,
IdlePool: cfg.IdlePool,
ConnMaxLifetime: cfg.ConnMaxLifetime,
ConnMaxIdleTime: cfg.ConnMaxIdleTime,
})
if err != nil {
return Transactor{Connection: nil}, errors.Wrap(err, "creating new connection")
}
err = pop.CreateDB(conn)
if err != nil {
log.Warnf("auto-creating database, got error '%v'", err)
}
err = conn.Open()
if err != nil {
return Transactor{Connection: nil}, errors.Wrap(err, "opening connection")
}
mig, err := NewEmbedMigrator(dax.MigrationsFS, conn, log)
if err != nil {
return Transactor{Connection: nil}, errors.Wrap(err, "getting embedded migrator")
}
err = mig.Up()
if err != nil {
return Transactor{Connection: nil}, errors.Wrap(err, "migrating DB")
}
return Transactor{Connection: conn}, nil
}
// DropDatabase drops the database associated with the given
// Transactor (which embeds a live database connection). This is
// destructive, you will lose data.
func DropDatabase(trans Transactor) error {
conn := trans.Connection
return pop.DropDB(conn)
}

View file

@ -6,7 +6,9 @@ import (
"database/sql"
"github.com/featurebasedb/featurebase/v3/dax"
"github.com/featurebasedb/featurebase/v3/dax/controller"
"github.com/featurebasedb/featurebase/v3/errors"
"github.com/featurebasedb/featurebase/v3/logger"
"github.com/gobuffalo/pop/v6"
)
@ -14,6 +16,57 @@ import (
// which can be used by the controller agnostic of implementation.
type Transactor struct {
*pop.Connection
logger logger.Logger
}
func NewTransactor(cfg *controller.SQLDBConfig, log logger.Logger) (Transactor, error) {
conn, err := pop.NewConnection(&pop.ConnectionDetails{
Dialect: cfg.Dialect,
Database: cfg.Database,
Host: cfg.Host,
Port: cfg.Port,
User: cfg.User,
Password: cfg.Password,
URL: cfg.URL,
Pool: cfg.Pool,
IdlePool: cfg.IdlePool,
ConnMaxLifetime: cfg.ConnMaxLifetime,
ConnMaxIdleTime: cfg.ConnMaxIdleTime,
})
if err != nil {
return Transactor{Connection: nil}, errors.Wrap(err, "creating new connection")
}
return Transactor{
Connection: conn,
logger: log,
}, nil
}
// Start creates the database specified in the database connection, then runs
// any outstanding migrations.
func (t Transactor) Start() error {
conn := t.Connection
// Create the database if it doesn't exist.
if err := pop.CreateDB(conn); err != nil {
t.logger.Warnf("auto-creating database, got error '%v'", err)
}
// Open a connection to the database.
if err := conn.Open(); err != nil {
return errors.Wrap(err, "opening connection")
}
// Run migrations.
if mig, err := NewEmbedMigrator(dax.MigrationsFS, conn, t.logger); err != nil {
return errors.Wrap(err, "getting embedded migrator")
} else if err = mig.Up(); err != nil {
return errors.Wrap(err, "migrating DB")
}
return nil
}
func (t Transactor) BeginTx(ctx context.Context, writable bool) (dax.Transaction, error) {
@ -44,3 +97,11 @@ func (w *DaxTransaction) Context() context.Context {
func (w *DaxTransaction) Rollback() error {
return w.C.TX.Rollback()
}
// DropDatabase drops the database associated with the given
// Transactor (which embeds a live database connection). This is
// destructive, you will lose data.
func DropDatabase(trans Transactor) error {
conn := trans.Connection
return pop.DropDB(conn)
}

View file

@ -7,6 +7,11 @@ import (
)
type Transactor interface {
// Start is useful for Transactor implementations which need to establish a
// connection. We don't want to do that in the NewImplementation() function;
// we want that to happen upon Start().
Start() error
BeginTx(ctx context.Context, writable bool) (dax.Transaction, error)
Close() error
}

View file

@ -10,8 +10,9 @@ import (
)
func TestDirectiveVersion(t *testing.T) {
trans, err := sqldb.Connect(sqldb.GetTestConfigRandomDB("directive_version"), logger.StderrLogger) // TODO running migrations takes kind of a long time, consolidate w/ other SQL tests
trans, err := sqldb.NewTransactor(sqldb.GetTestConfigRandomDB("directive_version"), logger.StderrLogger) // TODO running migrations takes kind of a long time, consolidate w/ other SQL tests
require.NoError(t, err, "connecting")
require.NoError(t, trans.Start())
tx, err := trans.BeginTx(context.Background(), true)
require.NoError(t, err, "getting transaction")

View file

@ -13,7 +13,22 @@ import (
type Node struct {
Address Address `json:"address"`
// RoleTypes allows a registering node to specify which role type(s) it is
// capable of filling. The controller will not assign a role to this node
// with a type not included in RoleTypes.
RoleTypes []RoleType `json:"role-types"`
// HasDirective will be true when the node has received at least one
// directive from the controller. This can be used to instruct the
// controller that it should send a directive regardless of whether it
// already knows about this node. This can happen in an on-prem, serverless
// setup (when the controller and computer are both running in the same
// process) and the node is restarted. In that case, the controller comes
// up, reads the meta data, and assumes that the local computer registering
// with it has already registered. But we really want the controller to
// treat this as a new node registration so the computer can load data from
// snapshotter/writelogger.
HasDirective bool `json:"has-directive"`
}
// Nodes is a slice of *Node. It's useful for printing the nodes as a list of

View file

@ -32,6 +32,10 @@ type ManagedCommand struct {
svcmgr *dax.ServiceManager
// Hang on to the Transactor so we can use it to drop the database upon
// closing the ManagedCommand.
trans sqldb.Transactor
started bool
}
@ -62,7 +66,14 @@ func (mc *ManagedCommand) Start() error {
// Close closes the embedded command.
func (mc *ManagedCommand) Close() error {
return mc.Command.Close()
if err := mc.Command.Close(); err != nil {
return errors.Wrap(err, "closing command")
}
// Drop the database upon closing.
// return sqldb.DropDatabase(mc.trans)
return nil
}
// NewController adds a new ControllerService to the ManagedCommands ServiceManager.
@ -196,6 +207,13 @@ func NewManagedCommand(tb fbtest.DirCleaner, opts ...server.CommandOption) *Mana
mc.Config.Computer.Config.SnapshotterDir = path + "/sn"
mc.Config.Controller.Config.SnapshotterDir = path + "/sn"
var err error
testconf := sqldb.GetTestConfig()
mc.trans, err = sqldb.NewTransactor(testconf, logger.StderrLogger)
if err != nil {
tb.Fatalf("getting new transactor: %v", err)
}
return mc
}
@ -231,29 +249,26 @@ func MustRunManagedCommand(tb testing.TB, opts ...server.CommandOption) *Managed
mc := NewManagedCommand(tb, opts...)
var err error
testconf := sqldb.GetTestConfig()
fmt.Printf("testconf: %+v", *testconf)
trans, err := sqldb.Connect(testconf, logger.StderrLogger)
require.NoError(tb, err, "connecting")
// Start the Transactor.
require.NoError(tb, mc.trans.Start())
// The integration tests reuse the same database every time, but
// truncate all the tables *before* the tests run (rather than
// after). This has the advantage that if the tests fail partway
// through, you can inspect the state of the database for
// debugging purposes.
err = trans.TruncateAll()
err := mc.trans.TruncateAll()
if err != nil {
tb.Fatalf("truncating DB: %v", err)
}
// The migrations contain an insert, but since we just truncated everything we need to redo that insert.
err = trans.RawQuery("INSERT INTO directive_versions (id, version, created_at, updated_at) VALUES (1, 1, '1970-01-01T00:00', '1970-01-01T00:00')").Exec()
err = mc.trans.RawQuery("INSERT INTO directive_versions (id, version, created_at, updated_at) VALUES (1, 1, '1970-01-01T00:00', '1970-01-01T00:00')").Exec()
if err != nil {
tb.Fatalf("reinserting directive_version record after truncation: %v", err)
}
err = trans.Close()
err = mc.trans.Close()
if err != nil {
tb.Fatalf("Closing conn after truncating all tables: %v", err)
}

View file

@ -125,6 +125,9 @@ func TestDAXIntegration(t *testing.T) {
mc := test.MustRunManagedCommand(t)
defer mc.Close()
computerKey0 := dax.ServiceKey(dax.ServicePrefixComputer + "0")
mc.WaitForApplied(t, computerKey0, 60, time.Second)
svcmgr := mc.Manage()
// Set up Controller client.
@ -521,6 +524,72 @@ func TestDAXIntegration(t *testing.T) {
})
})
// Ensure that restarting both the controller and the computer comes up in a
// usable state. Prior to the `HasDirective` member added to the `dax.Node`,
// if an on-prem process (made up of sub-services) restarted, the Controller
// would ignore the Computer registering because it already know about it.
// This ensures that they can be restarted and the Computer will receive a
// directive.
t.Run("All_Restart", func(t *testing.T) {
mc := test.MustRunManagedCommand(t)
defer mc.Close()
svcmgr := mc.Manage()
// Set up Controller client.
controllerClient := controllerclient.New(svcmgr.Controller.Address(), svcmgr.Logger)
// Create database.
qdb.Options.WorkersMin = 1
qdb.Options.WorkersMax = 1
assert.NoError(t, controllerClient.CreateDatabase(context.Background(), qdb))
controllerKey := dax.ServiceKey(dax.ServicePrefixController)
computerKey0 := dax.ServiceKey(dax.ServicePrefixComputer + "0")
// Ingest and query some data.
t.Run("ingest and query some data", func(t *testing.T) {
runTableTests(t,
svcmgr.Queryer.Address(),
basicTableTestConfig(qdbid, defs.Keyed)...,
)
})
t.Run("stop controller", func(t *testing.T) {
assert.NoError(t, svcmgr.ControllerStop())
assert.False(t, mc.Healthy(controllerKey))
})
t.Run("stop computer0", func(t *testing.T) {
assert.NoError(t, svcmgr.ComputerStop(computerKey0))
assert.False(t, mc.Healthy(computerKey0))
})
t.Run("restart controller", func(t *testing.T) {
assert.NoError(t, svcmgr.ControllerStart())
assert.True(t, mc.Healthy(controllerKey))
})
t.Run("restart computer0", func(t *testing.T) {
assert.NoError(t, svcmgr.ComputerStart(computerKey0))
assert.True(t, mc.Healthy(computerKey0))
mc.WaitForApplied(t, computerKey0, 60, time.Second)
})
// Query the same data.
t.Run("query the same data", func(t *testing.T) {
runTableTests(t,
svcmgr.Queryer.Address(),
tableTestConfig{
qdbid: qdbid,
test: defs.Keyed,
skipCreate: true,
skipInsert: true,
},
)
})
})
t.Run("Delete_Database", func(t *testing.T) {
mc := test.MustRunManagedCommand(t)
defer mc.Close()

View file

@ -314,12 +314,21 @@ func (m *Command) checkIn(addr dax.Address) {
m.logger.Printf("no Controller implementation with which to check-in on node: %s", m.Config.Advertise)
}
// Determine if this node has received at least one directive from
// the controller. This will be true if the server's Holder has a
// directive with a non-zero version.
var hasDirective bool
if holder := m.Server.Holder(); holder != nil {
hasDirective = holder.Directive().Version > 0
}
node := &dax.Node{
Address: addr,
RoleTypes: []dax.RoleType{
dax.RoleTypeCompute,
dax.RoleTypeTranslate,
},
HasDirective: hasDirective,
}
if err := m.Registrar.CheckInNode(context.Background(), node); err != nil {

View file

@ -15,5 +15,30 @@ var minmaxnegatives = TableTest{
srcRow(int64(3), int64(33), int64(-33)),
),
),
SQLTests: []SQLTest{},
SQLTests: []SQLTest{
{
// Select all.
name: "select-all",
SQLs: sqls(
"select * from minmaxnegatives",
),
ExpHdrs: hdrs(
hdr("_id", fldTypeID),
hdr("positive_int", fldTypeInt),
hdr("negative_int", fldTypeInt),
),
ExpRows: rows(
row(int64(1), int64(21), int64(-21)),
row(int64(2), int64(32), int64(-32)),
row(int64(3), int64(43), int64(-43)),
// TODO(tlt): this test did not exist, and the values coming
// back are incorrect. We need to fix this test based on the
// correct results below:
// row(int64(1), int64(11), int64(-11)),
// row(int64(2), int64(22), int64(-22)),
// row(int64(3), int64(33), int64(-33)),
),
Compare: CompareExactOrdered,
},
},
}