So we have a problem which is triggered in part by the race detector,
but which is actually deeper, but also possibly rare enough to be
politely ignored.
The real underlying issue is that sometimes when we have multiple
tests running in CI, multiple instances of the CLI test end up using
the same postgres database backing for some of their DAX stuff. We
have workarounds for this in some places, but not others.
But the *observed symptom* of this is that it can cause a trivial
race detector issue where we have one call to `(*Resource).Lock()`
and another call to `(*Resource).IsLocked()` which aren't synchronized
in any way, so if the race detector spots this, it complains.
We can suppress that very easily by synchronizing these. That does
not solve the other possibly-weird problems, so this may not actually
address the issue, but I think it might reduce the rate of sporadic
failures significantly, which would give us some time to think about
solving the deeper problem.
The underlying design issue is that we're reusing the database name
in postgres for testing. This lets us have bounded growth (one database)
while leaving the database contents up after a failed test (so we can
examine them), then truncating the database during startup if it already
exists. Which works fine if *only one thing runs at once*, which would
be true on a laptop, but in CI, it's sometimes not true. A real fix
for that is complex and requires some rethinking of how we approach
the test stuff, as we don't want unbounded growth, but we also don't
want two copies of the test running at once to see each other, and
ensuring cleanup after a test failure is surprisingly hard.
Switch Serverless from using BoltDB to Postgres as metadata store.
Previously, the controller stored all metadata to BoltDB. This implements SQLDB (currently Postgres flavored) as the backing store for metadata. This will allow us to have multiple instances of the controller running for HA, and to easily inspect and repair the contents of the metadata store.
Unfortunately, it was not straightforward to keep the BoltDB implementation working alongside the SQL one, so it will be removed in a later patch. Once that's done, the SQL implementation should allow for a number of simplifications of the schemar and balancer interfaces.
Database migration is built directly into the application by embedding the migration files and logic from the `soda` command line tool. When connecting to the RDBMS, the app will always attempt to create the necessary database and apply any outstanding migrations.
Integration tests truncate all tables upon start, but *not* at the end, so the state of the database can be inspected after integration tests.
Had to refactor some of the controller's background tasks to make sure they get properly shut down on controller exit.
* Implement Schemar.SetDatabaseOption(option, value string)
This replaces the temporary `SetDatabaseOptions()` method, which
replaced the entire DatabaseOptions struct, with `SetDatabaseOption`
which takes an option/value pair of strings to set.
* Add SetDatabaseOption to controller http handler and client
This commit also:
- renames some `writeLog` to `writelog`
- updates ApplyDirective to call resource.Unlock() on any resources
being removed from the local worker
* Add Database related methods to SchemaAPI interface
Currently all implementations of this interface are implemented with
"unimplemented" errors on those methods. Next will be to implement the
necessary methods.
* SQL: CREATE DATABASE and SHOW DATABASES
* SQL: DROP DATABASE
* SQL: Add UNITS option to CREATE DATABASE
* SQL: ALTER DATABASE
* User serverlessStorage.Remove[*]Resource instead of resource.Unlock()
* Add WITH keyword to CREATE/ALTER DATABASE
* fix some WITH logic
* linter fixes
* WITH on CREATE DATABASE is not required
* Support Drop Table in serverless (include Snapshotter, Writelogger)
* Finish Database methods
Things like:
- `Databases`
- `DatabaseByID`
- `DatabaseByName`
- `DropDatabase`
* Change Poller to use NodeService instead of its own map
Instead of the Poller maintaining its own map of Addresses to poll, this
commit changes the Poller to use the NodeService interface to get all
known nodes from the Controller.
The next commit needs to:
Next, the logic in the boltdb NodeService implementation was moved to
the boltdb Balancer implementation. That way, the Balancer can be the
source of truth for all things nodes/workers/jobs.
* Move NodeService from Controller to Balancer
This commit moves the implementation of the NodeService into the
Balancer, and aligns `Balancer.AddWorker` with `NodeService.CreateNode`
so that they stay in sync. (Same for `Balancer.RemoveWorker` and
`NodeService.DeleteNode`).
* fix import of private repo
* Fix go vet issues
* Fix bug in DeregisterNode
We need to remove the node from the NodeService even if it's not
assigned to a database. The logic had a bug in it.
This also adds some no-op implementations for SnapshotService and
WriteloggerService. If a directory was not configured for that, then the
computer node would panic on trying to read from the Snapshotter upon
receiving a Directive.
* queryer response content-type: json
* Add support for NULL to WriteloggerDir and SnapshotterDir configs
This commit changes the way WriteLoggerDir and SnapshotterDir are
handled.
If value is empty `""`, an error will be returned on computer startup.
If value is `"NULL"`, a no-op implementation of the service will be
used. This would be for a case that wanted to run serverless on-prem
with no durable storage.
Finally, any other value will be used as the directory to use.
Some things which aren't considered here and may result in unexpected
behavior:
- a value with spaces `" "`
- any "null" which is not "NULL"... like lowercase.
* Finish the DropTable test
* Change "disable service" value to case-insensitive "off"
This commit also removes an unnecessary sleep in the tests.
* Fix docker-compose variables for IDK test
Co-authored-by: Matthew Jaffee <jaffee@pilosa.com>
* Database isolation: Balancer
Remove naive Balancer
remove debugging lines
Thread dax.Transaction through Controller
Change role to roleType
Swap out Balancer interface with new one
Standardize InvalidTransaction error
Add some interface comments
* Remove type.Worker; replace with type.Address
* Remove database validate from Queryer
This is already being handled in the `CreateTable()` method. Prior
to doing that validation, we were getting a panic, but that's no longer
the case.
* Remove dax.TableQualifier; replace with dax.QualifiedDatabaseID
* Update IDK test to create database
(cherry picked from commit d971cfc269)
- had to make sure we don't snapshot until directive is fully applied
on a computer... otherwise there's races between loading the files and
truncating the write log.
- added a dirty bit to resources and a bool return to incrementing the
write log... don't snapshot if it returns false because that means
there's been no writes. (but make sure you close the storage transaction!)
- added the actually snapshotting routine which just fires every
<timeout> and serially snapshots everything.
- tweaked some logging
- added ability to get all tables in an org/db or literally all. I
think I just needed the "literally all", but it was natural to allow
it to be scoped to org or DB as well.
(cherry picked from commit b8b08bc9eb)
- check that serverlessStorage is not nil before closing it
- check that we don't already hold a lock on a serverless storage
Manager before trying to load it. This fixed at least one test failure.
(cherry picked from commit 87d1c31607)
