Commit graph

14 commits

Author SHA1 Message Date
David Kagan
24a45bc30d
Cloud 1475 (#2371)
* working on incorporating regex logic

* Implemented a validation check for database name with given rules in doc within controller

* fixing tests to pass

* reflecting changes to match docs

* fixed tests further, hopefully

* for sure fixed integration tests, and moved validation check

* integration tests passed, go test now will pass

* fixed name size to 230 due to previous commit acknowledgement

* fixed field test negative validations

* added missing comma
2023-04-07 15:08:07 -04:00
Travis Turner
ea72396b4d
Remove Node from data model; standardize on Worker (#2366)
* Remove Node from data model; standardize on Worker

This commit does a lot of things, but in general it attempts to simplify
the data model by getting rid of the Node and NodeRole models. Instead,
these will use the Worker model, which itself has individual boolean
fields for role types.

Get rid of roleType in some FreeWorker methods

rename NodeService to WorkerRegistry

simplify the freeworker interface

fix the tests

* Remove DeleteWorker method from workerJobService
2023-04-04 20:20:53 -05:00
Travis Turner
8fca15e936
RetryWithTx (#2348)
* First pass at RetryWithTx

* Refactor RetryWithTx to take a writable bool (instead of reads, writes)

* Implment DirectiveMethodDiff

This commit adds support for a Directive to contain only the diffs (as
opposed to the full Directive).

* Update controller tests to allow for DirectiveMethodDiff (over Full)

* Update RetryWithTx to retry on duplicate key constraint.

If two concurrent processes call IngestShard() for the same shard, both
were trying to insert the same job into the jobs table. That resulted in
a duplicate key error from the database. We want to include that error
in the list of errors for which RetryWithTx should retry.

* Remove unused method: Directive.TranslatePartitions()

* Replace query in a loop with a single query

We had a query which was looking to see if a job already existed. That
query was inside a loop, and could potentially generate 256 queries (for
example). This commit replaces that logic so that we use a single query
wiht an `IN ()` clause.

* Convert to directive version-by-address

This commit uses a separate directive version per address. It moves the
version get/increment back inside the buildDirective method so that if
two concurrent processes are building a directive for the same address,
one of them will get rolled back trying to commit the version update.

* Migration for directive version by address

* Add a comment about DirectiveVersion lock/unlock logic

* Remove AddLastWins

* fix linter

* handle error in walkdir

* fix test failures from removing AddLastWins
2023-03-30 20:54:37 -05:00
David Kagan
dd90838deb
Cloud 1457 (#2347)
* added dupe check

* added dupe check for databases

* prevent dupe table names

* refactored table dupe search

* further refactoring

This commit, further refactoring of checking for duplicates are done with SQL commands
Also adjusted tests to confirm changes

* fixed linting errors

* moved the errors around to keep them consolidated in the dax package

also removed useless comment
2023-03-23 16:20:45 -04:00
Matthew Jaffee
8fe73146c8
Sqldb rip boltdb (#2341)
* serverless sqldb use same env for test config as normal

* rip boltdb implementation of controller backend out

it was replaced by postgres and no longer works properly.

This involved migrating a number of tests which only worked with
boltdb, which exposed several ways in which the postgres
implementation had slightly different behavior from the bolt
one:
1. ordering of results in some cases, and
2. (more importantly) erroring when a record to delete was not
found. The bolt implementation silently ignored it when things to
delete weren't found, so we make some changes to match that behavior.

Also stopped propagating CreatedAt and UpdatedAt from DB tables into
dax types. These were breaking existing tests. Perhaps it would be
better to actually use them, but for now they will only exist at the
DB level.

This change set also moves the insertion of the directive_versions
record out of migrations and into the startup/connection code. Having
this in the migrations was a bit ugly because you couldn't just
truncate all the tables and have everything work from
scratch. Inserting it during startup is fairly innocuous, and will
just continue on if it already exists.

* update directive_version test

I changed the initial value to 0 so that the first version that gets
sent out is 1
2023-03-22 08:54:13 -05:00
Travis Turner
2f7ae30784
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.
2023-03-21 13:22:16 -05:00
Matthew Jaffee
be4f365eaf
Cloud 1358 bolt postgres (#2286)
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.
2023-03-20 09:02:22 -05:00
Seebs
244d80753e reuse clients instead of making new clients
Buckle in, this one's a ride.

This is attached to the same PR as a fix for exiting abruptly
during some tests because I ran into that issue, and comprehended
it, while trying to track down weird and sporadic test failures
that were actually this issue.

The actual, underlying, problem: `make test`, by running all the
tests at once, was hitting a bug that was mostly effectively
triggered by running the `dax/test/dax` tests, and the top-level
`featurebase/v3` tests, at the same time. However, the interaction
was nothing as obvious as temporary files, etcd configuration,
or whatever.

We were running out of port numbers.

The tests were using a bit over 30k simultaneous established TCP
connections, each to different ports, because we were creating
new clients for basically every single operation. For instance,
in a single SQL test that did an import and then a read, we
were creating a new client for each field written to, and then
also creating a new client for each field in results that needed
key translation. And none of these clients were closed or
timed out in any way. In fact, Go doesn't really *do* "closing"
of clients; the closest is that an http.Client can be told to
close idle connections that it has been keeping open.

The worst offenders were both named `fbClient`, and were nigh-identical,
except one of them was implemented as a method on `importer` in the
IDK tree, and one was a standalone function.

It may seem surprising that the method on `importer` is using a shared
client pool for all importers, rather than a new pool for each
importer. This is because we potentially make quite a few importers
during tests.

Before this, running either of the dax tests or the top-level
tests would show well over ten thousand simultaneous ESTABLISHED
connections. After this, the dax tests used nearly twenty.

The problem with port consumption like this, while more noticeable
on MacOS, is also something we could hit on the CI runners, especially
if a single runner ended up with more than one test suite running
at the same time. This probably manifests as sporadic very strange
failures of CI, with messages about "cannot assign requested address".
(Note that an outgoing connection to a successfully-created port
requires *another* port to be assigned for the outbound socket.)

This was complicated dramatically by the fact that, for some
utterly cursed reason, it was *especially* common for the point
at which we hit this, in the top-level featurebase tests, to be
running one of the backup tests in TestVariousQueries, and
specifically, to be hitting it on the dataframe part of the
backup... Which is to say, on the *one* path in the backup function
that called log.Fatal, and thus terminated the featurebase process
abruptly without further commentary.
2023-03-06 13:12:22 -06:00
David Kagan
d7c6258f16
Cloud 1359 errors across http (#2279)
* WIP: json marshal coded errors for http

* WIP: trying to see how best to implement the http-error tests

* finish stubbing out the Schemar methods in the test

* using json to move CodedErrors across boundaries and associated tests

* implemented feedback and fixes

---------

Co-authored-by: Travis Turner <travis@molecula.com>
2023-02-27 16:34:51 -05:00
Matthew Jaffee
41a6b9e823
controller commit to DB first then send directives (#2259) 2023-02-17 09:12:15 -06:00
Travis Turner
87011e4294
CLI: make it more like psql (#2235)
* Refactor CLI to mimic psql's meta-commands

This PR adds support for meta-commands (also known as "backslash
commands") like those in psql, Postgres's CLI. Only a few meta-commands
are currently implemented, but this was meant to demonstrate how we
could use something like `\i file.csv` to insert local files into SQL
statements.

* Meta-commands: \file and \include

The initial implementation used `\i` as a streaming file handle.

This commit changes that to `\file`, and then implements `\i` (or
`\include`) as handling multiple sql commands.

* Add meta-command "help" (\?)

This is basically a copy of the psql help output, but includes only
those options we currently support.

* Add support for \o [file], and \timing

The \o meta-command writes query output to a file.

The \timing meta-command turns on/off the timing display sent to stdout.

* Add meta-commands: \l (show databases) and \dt (show tables)

* Add meta-command: \watch [period]

* Update meta-command \connect to take database name instead of ID

* Add support for \echo, \qecho, and \warn

This commit contains an known issue in that the `-n` option will exclude
the line feed, but if the output is the terminal, the readline package
clobbers any content on the current line (i.e. anything without a line
feed). That will need to be addressed at some point.

* Add support for \w [FILE] (write query buffer to file)

* Add SchemaAPI no-op implementation

* Refactor query handler to align with /sql and /databases endpoints

We want to standardize on:
/sql
/databases/{databaseID}/sql

* Add CLI support for expanded, border, tuples_only (and pset)

* Add help text for \pset and \t
2023-02-14 09:23:51 -06:00
Travis Turner
126be915a9
Support for setting individual DatabaseOptions (#2231)
* 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
2023-02-06 08:50:28 -06:00
Matthew Jaffee
903e234c69
tweak a bunch of logging and config (#2234)
* tweak a bunch of logging and config

make overall logs less verbose and chatty

1 minute computer check-in interval

3 minute snapshot interval

remove CaptureLogger as it has same functionality as buffer logger

add a WithPrefix to the Logger interface so sub-services can have
different prefixes

* fix some lint

* fix lint... confused why this is coming up now
2023-02-03 14:59:07 -06:00
Travis Turner
20429bb9dc
Remove MDS and replace it with Controller (#2219)
* Remove MDS and replace it with Controller

This commit removes the MDS layer (and package) and shifts Controller
package into its place.

* add pprof/fgprof to serverless http router

---------

Co-authored-by: Matthew Jaffee <jaffee@pilosa.com>
2023-01-30 16:54:12 -06:00