If we can't successfully generate a PQL Percentile call, error
out rather than implementing an actual Percentile function in SQL.
This can be revisited if anyone needs it.
So there's a lot going on here.
Percentile just did not work, even a little, with decimals.
In theory we try to make the int val part of ValCount work, in
ValCountize, but you can't actually use that for everything because
it unconditionally adds bsig.Base even when it shouldn't. But it
doesn't matter that we were returning those values from, say,
(Field).MinForShard, because ValCount.Smaller was not preserving them
when identifying the smaller of two Decimal ValCounts anyway.
And even if it did, the logic in Percentile wouldn't have worked
with passing the raw unscaled integer in as a value to compare
against.
But that's fine because the logic was also more generally wrong.
According to the existing logic, a value is the median value if
exactly as many values are less than it as are greater than it.
This is... not actually very accurate to what we usually mean by
"median". Because some values are *equal* to a given value. So
for instance, say you have the values {1, 1, 1, [a million 2s], 3}.
Our logic would regard 2 as being too high to be the median, because
3 times as many values are lower as are higher.
New interpretation: Imagine a sorted list of all your values, with
N entries. You want the Nth percentile, which is to say, you want N%
of values to be less than the vale you pick, and (100-N)% to be greater.
You can round both of these down. So for instance, if you have 6 values,
and want the median, you want 3 values greater, and 3 values less. To
be picky, we could demand the average of those middle two values, but
we're not in a good position to do that in this implementation.
If the number of desired things less than, or greater than, a target
is 0, we can short-circuit to the minimum or maximum value. This can
happen when nth is close to an end and the number of things is small,
not just at nth=0/nth=100.
So we rework this, and we rework the tests for this behavior to reflect
that logic.
We change executePercentile to be able to return a nil rather than
a weird ValCount in cases where there's no result, such as when
there's no values to compute a percentile of.
We also change the SQL tests to match the new behavior, since some
of them were expecting everything done on a decimal field with values
10-13 to come back as 10.00 as a decimal because that is what the
code returned.
We also propagate these changes to DAX, and along the way, fix up a
TODO item in the DAX copy, and stop skipping the test that was
failing because of that TODO item.
It's reasonable to allow "where x > 13" on decimal
fields. Handle at least int64 and float64.
Once this is up, we find that aggregates can return
non-values, such as nil, in some cases; for instance,
`percentile(x) where x > 13` can yield a nil if x is
never greater than 13, rather than making up a value
from zero data points. So we accept nil as a valid
result type in PQL aggregates.
As a result of this, change two tests which were
unintentionally testing for an arcane edge case bug
in which (1) we can't render a condition to PQL,
such as because you specified an integer for a decimal
field, and (2) the filter is using an aliased name,
in which we would end up failing to generate a PQL
filter, but *also* losing the SQL-layer filter, and
produce wrong results as though there were no filter.
We also alter the tests to use `o.price > 9`, because
this lets us generate three user names, but only two
distinct user names, so the test using DISTINCT returns
a different value than the test not using DISTINCT,
which helps us verify that it's actually working and
not just lucky.
As part of fixing that, there was an intermediate
state where we rejected as an error any case where
generating the PQL filter failed. This broke 21 more
test cases, but in all of those cases, the SQL filter
was actually working.
... But in two of them, we SHOULD have been able to
generate PQL, because they were testing bools for
null, which works fine. We just had a list of
field types we allowed null tests against and
omitted bool because I forgot that bool isn't always
just treated as a kind of mutex.
When pushing an expression down to PQL Percentile, if we have
a filter, it has to be passed as the argument "filter", not as
an additional child argument. We don't need to pass in `All()`
as a filter if there's no filter, Percentile works fine with
no filter provided.
* 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
* tidy up show tables behavior
* made cli integration test whole again
* Update fbsql \d meta-command to show system tables (#2376)
---------
Co-authored-by: Travis Turner <travis@molecula.com>
We forward-ported a handful of tests from the previous parser
which relied on LIMIT clauses, but then we didn't support that.
Now that we do, we uncomment most of these tests, and actually
give them the correct data structures to compare with.
We leave two tests commented out. One was using `limit 10, 5` to
express a limit plus offset, and the other is using `not fld = 1`
as a WHERE clause, but we don't support unary-not to negate
other expressions.
In the process, we discover that converting a SELECT with a
LIMIT clause back to a string has a missing space, and fix that.
analyzeExpression - tupleLiteralExpression was covered by something
else between the ticket getting filed and me starting on it.
(*ExecutionPlanner).analyzeBinaryExpression now has increased
coverage for IN / NOT IN. Several bugs got revealed by adding tests;
those tests are commented out but can be re-enabled by whoever ends
up working on the bugs. Tickets are filed.
* 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
The attempt to set the TrackExistence option for fields
happened before checking whether the field was created
successfully or not. Credit to Rachith for spotting this.
Bug was introduced with the TrackExistence stuff, but
we apparently never had a test case for invalid min/max
values.
parseOperand was assuming that any reference to MIN in a place
where an operand was expected was a call, which it should be,
but it might not be. parseCallExpression panics if it doesn't
find a parenthesis, because it's never supposed to be called
when we don't know we have one.
The test for this is in with MinMaxColumnConstraints, even though it's
actually a test of MinMaxFunctionCalls, because that's where the other
tests involving the special MIN/MAX tokens live.
We also stop checking whether MIN or MAX might actually be QIDENT.
If you use a quoted identifier, we're over in the QIDENT case,
not the MIN/MAX case. If the token was MIN or MAX, it's always
unquoted.
* Add test coverage for executionplanner.go
*ExecutionPlanner.mapper does not get tested in the case where its
context gets cancelled. In order to make testing this possible,
I've added a context argument to sql_test.MustQueryRow. If it's
nil, MustQueryRow creates a context for itself just like it always
has, but if a context is provided, it uses that.
* Adds test coverage for ExecutionPlanner.mapper in executionplanner.go
The case where the context gets cancelled mid-query is now covered.
The test is timing-dependent - the cancel call has to happen after
the query has been started but before it finishes, and in just the
right part of MustRunQuery, in order to actually produce a context
cancelled error, and not, say, a query cancelled error. May have to
adjust timing if the current delays don't work in CI testing.
* Addressed review notes
-reordered arguments for MustRunQuery
-moved MustRunQuery out of a goroutine, put the cancel in one
*Tests are added to extend coverage for statement, expression and source types and many of the ast helper functions
*For those SQL language elements where ast exists but parsing is not implemented, test coverage is added to test only the ast correctness
*Also, removed timestamp EPOCH related compiler code as they become unreachable after their ast equivalent were removed in a previous PR.
* 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
When doing the release process, we generate version numbers
that have a version tag but don't have a hash. The IDK test against
the expected hash doesn't work in this context. Let's check for an
expected tag first.
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.
Assignment compatibility checking in analyzeBulkInsertStatement is
now tested. This isn't checking the values themselves, it's there
to make sure the structure is correct for mapping values to columns.
It turns out that the problem with nested joins was that we were
trying to cleverly invert them, but that seems to be incorrect and
resulted in incorrect nesting.
The test case for this is
SELECT * FROM X INNER JOIN Y ON true INNER JOIN Z ON false
this is now parsed as
(X inner join Y on true) inner join z on false
Which, as it turns out, is the structure that stringizes back to the
original statement.
We were previously parsing it as
X inner join (y inner join z on false) on true
which stringizes out to a different form, and is also, I think,
just straightforwardly not what we want.
So basically, we had special case code to recognize that we
were doing a join on top of another join, and invert them in
some way, and I have no idea why because that seems not to be
correct, or at least, it produces nonsensical stringizing that
we can't then parse.
We now test the tuple-assignment at all, although it's
perhaps confusing because we expect a ()-list of columns
to go with a {}-list of values. We also test a lot more
errors and some more successes, and additional literal types
in mustParseLiteral.
This is a sanity-check after a weird CI failure; we want
to ensure that we're actually getting the expected version of
featurebase. The environment variable here is magic to the
IDK tests.
We have had some weird problems that look like IDK was being tested
against the wrong version of featurebase. Add a test which requests
the version, and if an environment variable is set, requires that
the featurebase server agrees with it.
FB-2045
aggregate{Avg,Min,Max}->Update now tested for DataTypeDecimal.
{avg,min,max}PlanExpression->WithChildren now tested.
percentilePlanExpression->{Evaluate,Plan,WithChildren} is not
tested because percentile gets sent directly to PQL rather than
getting planned and evaluated in SQL.
aggregateLast->everything is not tested because Last is not yet
completely implemented.
We distinguish between "TrackExistence option is set"
and "we are actually doing existence tracking", to avoid
mishaps like accidentally creating an "existence" view for
a BSI field or something like that. This logic was being
done probably-correctly in one place, and ignored or
handwaved in some, so this is an attempt to just make
it more consistent.
We had two different versions of this, and a comment referring to a third
which doesn't exist, so I've consolidated them and made them slightly
pickier, to avoid problems like the one I ran into developing the existence
tracking where one of these optimistically transformed names it actually
shouldn't have. Now if we don't expect a view name, we yield an error,
rather than silently performing a transformation.
This also implies updating the ImportRoaring_MultiView test to use
two valid view names.
There's a lot going on here. First, we were treating "the test is
a Condition" as implying BSI, which it doesn't anymore. Second, the
behavior of conditions was weird and BSI-specific. Third, we had
to propagate these changes and features throughout a bunch of code,
including both the core featurebase code and the DAX replacements/copies
of it, plus the SQL3 layer.
We refactor this so that tests for equality and inequality work for
non-BSI fields, so now if you accidentally use `==` in a Row call
on a non-BSI field, it still works; that's not specific to BSI
fields anymore.
We add a TrackExistence flag to fields, and propagate it through
things like our protobuf code, etcetera, so that we can successfully
create fields. Newly-created fields get this by default, because
we add it unconditionally to them, but the paths that are being
called with existing fields don't add it. So, when we "create"
(really, just load the definition of) a field from something stored
in the schema, we don't add TrackExistence to it, but any path to
creating a new field should.
A time quantum field with NoStandardView will *effectively*
lack TrackExistence.
For sets, mutexes, and time quantums with a standard view, anything
that sets bits will also set a corresponding bit for the record in
a new "existence" view. This allows us to distinguish between an
empty set and a null, and also allows null checks to be constant-time.
When clearing bits, we don't clear existence bits EXCEPT that if
you clear a bit in a mutex, *and the bit actually existed*, we clear
the existence bit. For sets and time quantums, clearing bits never
clears the existence bit.
Deleting records clears the existence bit.
We also add code to the `batch` subpackage to generate suitable
existence field bitmaps and import them. This logic correctly handles
empty sets and nils. The `batch` package does not allow specification
of anything equivalent to clearing a single bit from an existing
record, so we don't have to deal with the mutex complexity in that
case, which is good because it would be impossible.
This requires a number of other subtle changes, such as allowing
new fields to have more than one FieldOption specified for them.
We also drop the handful of implementation bits relating to the
"fullySorted" internal-use-only import flag, which existed only to
support the JSON ingest API, which we've removed.
The most dangerous part of this is that the mutex semantics are
impossible to implement on top of our existing API, because they
require us to know, not how *many* bits we cleared, but which
*specific* bits we cleared. I've implemented this as a new Tx method,
which is almost certainly going to be tech debt one day; if we some
day drop the Import API, we should remove that.
The testing for this is only currently covering the Set/Clear
behavior of PQL, and the Import API. The batch tests haven't been
written yet.
Fields that don't have existence tracking enabled refuse to perform
null/not-null tests. They should also report themselves as having
no null values -- if a record exists, sets in it are considered
empty rather than null.
The SQL3 support requires a number of subtle modifications to both
featurebase and some addon tooling. The essential thing is dropping
the unconditional translation of nil slices to non-nil empty slices
in translateResult, both in the executor and the orchestrator. We
also modify the logic that handles generating results from Extract
calls, to ensure that non-null sets get an empty slice created for
them even if they never have any values assigned.
The expected results for some tests are different now; we expect to
get nil slices, rather than 0-length non-nil slices, for fields which
were never written for a given record. Most tests were not changed.
(In every case, if a test was failing, I actually checked the logic
before changing expected results. This required a lot of tracking down
of edge cases.)
The batch package now rejects as an error attempts to clear single
bits from mutex fields, because so far as I can tell it's simply
impossible to have a roaring import that specifies the correct semantics
there; you can't tell whether to clear an existence bit without
access to the currently-set bits, which the batch API doesn't have.
We already supported the special case of specifying a clear value
of nil for clearing a mutex field; now that is the only allowed
value for a mutex field to have in row.Clears.
We change the logic for fixing up incoming view names (in two places)
to stop assuming that any view in a time field other than "" that does
not have viewStandard as a prefix is a partial time quantum name that
should have "standard_" prepended to it. This allows us to submit
bitmaps for "existence" to time quantum fields and not have them
silently transformed into "standard_existence" because that's what we'd
do with "202203".
We drop the field ClearBits method, which was totally unused.
We drop the sliceDifference function, which was used in a previous
mutex implementation and hasn't been used in ages, and the test
case for it, and the helper function used only by that test case.
At some point in some other refactor, this option got removed from the
function call it was supposed to be an option to.
But actually that was... not correct either.
Because if you look closely, it turns out that this test was completely
broken; we were ignoring the results that were in the test, and using
inline results, but that's okay, because we were also doing the wrong
query for the second test, and ignoring the Field Options specified in
the test... all fixed now.
* 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
FB-2041
Added tests for typeIsTimeQuantum and typeIsSet and made them pass.
Added tests for DataTypeTuple cases in typesAreAssignmentCompatible.
Timestamp conversion checking is handled before it gets to that
point but I left those branches in as a backstop.
Checking to see if DataType[String,ID]SetQuantum can be assigned
to themselves doesn't appear to be reachable currently but left
those branches in, because something may use them in future.
Added one test to the DAX skip list since it's the IDSetQ version
of a StringSetQ test that was already on there, changed skip list
to refer to both tests by name instead of by number.
* 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
* fbsql disconnect from database with `\c -`
This adds the ability to disconnect from the current database by passing
a hyphen to the `\c` meta-command.
* Update cli/cli.go
Co-authored-by: Matthew Jaffee <jaffee@pilosa.com>
---------
Co-authored-by: Matthew Jaffee <jaffee@pilosa.com>
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.
DAX now relies on Postgres, so we use the Postgres which is already in
the IDK tests (for the external lookups thing) to use as the
controller's metadata store as well. The env variables are a little
confused, but I'll clean that up separately.
There's no actual way to forcibly sequence our check of the history
until after the history has been updated, but it's pretty fast usually,
just not always instantaneous. Without this, adding a few millisecond
delay in the tracker reliably produces the test failures we kept seeing
with an unexpectedly low length of 3. With this, it passes consistently
even with the artificial delay.
The query tracker being asynchronous is probably fine, but we need
to test it as though it might take a while.
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.
This is the time being used in our other CI pipeline.
The alternative to doing this is ripping out all of this code so that
we're not running two CI pipelines...
* renamed 2 system tables
* adding table column for types
* added a type field to fb_database_nodes system table
* updated ClusterNode struct
* adding backwards compatibility
this commit also adds support for ordering systemTables and implements the method
* fixed linting
---------
Co-authored-by: Travis Turner <travis@molecula.com>
* Enable linter: stylecheck
This enabled the stylecheck linter, but excludes some staticchecks for
now. The following are ignored because they will take a bit of time to
address, but the intention is to address them and remove them from the
exclusion list.
ST1000: at least one file in a package should have a package comment
ST1003: golang naming standards
ST1008: error should be returned as the last argument
ST1016: methods on the same type should have the same receiver name
ST1020: comment on exported function
* Address ST1015
For some reason this failed in CI but not locally. I can't figure out
why that check isn't happening locally. This just moves the switch
statements around so that the `default` is the first (or last) item.
* Adjust error string in test to match case-adjusted error
* Remove TestCloseTimeout
* stub out a test framework for fbsql
* Introduce fbsql integration test framework.
This also adds support for `pset location` to set the geo location (i.e.
time zone) in which timestamps should be displayed. And it adds support
for comments (lines starting with `--`) in the line reader/splitter.
* Replace Stdin and Stderr with setters and un-export them
* Add (commented out) linters that we should introduce
I went through the available linters and added (commented out) the ones
I think we should work on in the near term. In other words, fix them,
then uncomment them so they are enabled in CI.
* linter: errchkjson
* linter: ineffassign
* linter: gosimple
* linter: errname
FB-1897
The specs for the datetimepart function are, as far as i can tell,
identical to the existing datepart function. Per Pat, replaced the
datepart function with datetimepart rather than just adding
datetimepart as an alias. Made sure existing tests that were using
datepart got switched over.
Second go at this after sorting out weirdness with git.
Until we have API token support, connecting to cloud requires the
cognito configuration. This commit adds the production cognito settings
for defaults, and we have intentinally omitted these from the documentation.
FB-1896
Added the datetimename function, which returns parts of a timestamp
as strings. Month and day of the week are named ("January", "Monday")
while others are returned as a string of digits ("2023").
Added tests to the test definitions.
* Set fbsql prompt based on the connected database
This also changes the prompt to align with psql, where it begins with:
db=#
and the mid looks like:
db-#
* Require organizationID in on-prem, serverless queries
* Support meta-commands in `--file` command
I'm not sure why this was restricted before. Just an oversight.
* Change default history file name to fbsql_history
* Add port short flag: p
* Support meta command \list (for \l) and \out (for \o)
* cleanup while writing docs
* Have \cd with no arguments change to home directory
* Avoid shadowing `action`
* Add kafka support to CLI (fbsql)
This commit adds the ability to provide a `--kafka-config` command line
argument referncing a toml file to configure kafka.
* Move "Molecula Consumer" message to the logger; hide it in basic mode
* Fold decimal(scale) into kafka.source-type
* Build fbsql with cgo in docker for CI
* Re-organize the fbsql kafka config and setup.
Allow field config to use the table schema if no fields provided.
* Display timestamp fields with format RFC3339Nano
* remove kafkaRunner (no longer used)
* Fix cli/batch test (and make sure it's not excluded from CI)
The logic in our Makefile was exluding from tests any package with
`/batch` in the package name. This excluded `/cli/batch`, which is not
good.
This commit changes the exclusion logic to include the `/v3` portion of
the package name, so `/v3/batch`.
* Rename Basic() to SetBasic()
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.
The testing package is full of subtle magic, and one of the most
subtle is this: t.Logf, etcetera, all write to a buffer which is
then displayed after the test is run. Which means that, if you
exit, the buffer is never displayed. This means that, if a test
case can fail in a way that causes an instant exit, you don't
hit defers, you don't get your log messages, you just get a mysterious
exit of the process.
We have two cases where backup commands were calling log.Fatal
instead of returning an error. The error in question is displayed
correctly and informatively if returned, so we return it.
We also have one case where we were using os.Exit to avoid a
deadlock. Instead, we make the thing that would deadlock
conditional on the test not having failed. In the event that
the test fails, we now print our failure message correctly,
then also report an unclosed cluster. That's fine.
* first time quantum queries working
* implement select from timequantum columns
* skip a dax test
* make linter happy
* addressed review feedback
* reverted over eager test elimination
* correct reference for `having count(*)`
It turns out that `having count(*) ...` was always treating
the count(*) as exactly 1. After studying this a lot, I noticed
that in fact, we correctly handle other counts. The reason is
that there's already code to recognize aggregates in `having`
clauses as matching aggregates that are being computed -- but
it only covers the other aggregate clause types, not the newly
added `countStarPlanExpression` from making `count(*)` work even
if there's no `_id` field.
We add several corresponding test cases.
* fix sum(a_decimal) type conversion
Added a test case for this, and also added a fix for it.
Underlying issue: qualifiedRefPlanExpression could end up
producing an int64 instead of a pql.Decimal, even though it
had expected type Decimal.
Originally this worked by politely converting an int64 to
a pql.Decimal in the Evaluate phase, but this was not ideal;
the real question is why it was coming out as an int64 at
that step. Showed this to Pat, who spent a while studying it
and produced a better fix.
* temporarily comment out test which fails in DAX
This forward-ports a number of tests from the previous SQL
implementation. The porting is approximate in a number of ways,
and not all tests are implemented/tested yet.
In particular, several tests are currently disabled because
we don't support `limit n` constructs.
The tests that were primarily tests of the parser have been
brought forward as parser tests. One of them has been altered
to add parentheses, because our parser interprets
fld1 between 1 and 3 and fld2 = 2
as:
fld1 between (1 and 3) and (fld2 = 2)
which is invalid, while the old parser apparently interpreted it
as:
(fld1 between 1 and 3) and (fld2 = 2)
We have not yet verified the SQL spec's requirements here, but
sqlite agrees with our old parser, not our new parser, so this
may be a regression.
The old tests expected an INNER JOIN to suppress duplicate
values. Our new code does not, which is consistent with other
SQL implementations. This is a change, but the old behavior
appears to have been wrong. (You can still suppress duplicate
values by specifying DISTINCT.)
In the previous implementations, a value like `count(*)` had
`count(*)` as its column name. In the new implementation,
it has an empty string as its column name.
Related to this, the prior implementation allowed you to
write
select age, count(*) from grouper group by age having count > 1
but the new implementationt requires that to be spelled as
having count(*) > 1
This is consistent with other SQL implementations, so I think
the new behavior is correct.
The behavior of SHOW COLUMNS and SHOW TABLES has changed, in
that the specific results returned are significantly different.
Perhaps more significantly, the old system spelled the former
query as SHOW FIELDS, rather than SHOW COLUMNS. This may be
considered a regression, in that `SHOW FIELDS` no longer works,
and we should consider whether any hypothetical users might
have been relying on the output of either of these. (I hope
not, the new output is much better.)
Some of the old tests (the ones in handler_test) were accommodated
by adding a couple of specific test cases to existing tests,
specifically:
* handling timestamp values with `Z` rather than `+00:00`
* a join with a WHERE clause referring to fields in both
source tables
We introduce a new "partial" comparison type, because there's
no way for a test of `SHOW TABLES` to contain a correct table
row, because `SHOW TABLES` includes timestamps from when tables
were created. I'm not sure this is the right way to do this.
We add corresponding changes to dax_test, because the DAX tree
tests against the SQL tests.
We change the returned types of field names and field types to
plain strings, ironically because DAX needs this -- the test code
in the DAX tree is getting them back as plain strings, rather
than as dax.FieldName and dax.BaseType.
The tests using `having` are commented out because they don't
seem to be working, a ticket has been filed for this.
Two of the tests that should return strings are instead returning
untranslated integer IDs, but only for DAX, not for the regular
SQL tests, and the `delete` test has been commented out for
DAX-specific errors. If we merge this, the next step is to
ticket those and address them separately.
While fixing a bug that log messages were ending up
in the output buffer for backups, we fixed up a bunch of
things to do with log messages and output for various
commands.
Due to a subtle oversight, this means that since we did
that, executor_test's `chkSumCluster` has been dutifully
printing `hash:blahblahblah` to os.Stdout, and returning
an empty string.
This also, indirectly, fixes a very strange behavior
we've had ever since then, which is that a lot of test
output silently disappears. The reason is probably,
although I haven't found the right code path, that we
were ending up closing os.Stdout.
The caching of test results means that we get instant cached results
rather than actually running tests in some cases, which is virtually
guaranteed not to be what we want. There's almost no cases where
this actually speeds a thing up validly, and a lot where it speeds a
thing up invalidly.
* 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>
* Removed support for EPOCH column constraint from TIMESTAMP SQL data type.
* Implicit conversion of integers to timestamp will treat the integer value as seconds since unix epoch.
* Add new ToTimeStamp(num, timeunit) SQL scalar function to help convert integer values to timestamp.
* Add meta-commands \set and \unset (for variables)
* WIP: first pass at variable replacement
* Use a mapReplacer instead of having Command implement replacer
* remove circular reference with variables
* remove the `replacer` interface; just have it be a struct
* use a lexer for variable replacement
* Separate the featurebase and fbsql make build targets
Using the same build target was problematic because they shared the same
flags. Since the `-o` output flag was used, the fbsql binary was
overwriting the featurebase binary.
* make sure the make package target builds fbsql
we want to be sure we delete the files we created during the
run even if the test run panics, but not files other runs
may have created also in /mnt/ramdisk.
So, we were special-casing creating a ramdisk, and setting a special
environment variable for it, for boltdb translate files, to improve
performance.
But actually, etcd and test cluster data and so on all go in $TMPDIR,
and if you move all of those also into a ram disk, you get way better
performance. But 2GB may not be enough for that.
So!
We unify on $TMPDIR, we bump the default size to 4GB, we make the
size configurable, and we stop using the name RAMDISK. This should
improve performance on MacOS significantly for `make test` and
things like it, and also simplifies our lives by not having a special
case for the boltdb translate files.
Also change the environment variable names used in our CI config.
(I don't see where we mount the ramdisks, but I think that's happening
in our setup.)
* Store version-check file in the configured data-directory
This also fixes what I think is a bug.
It also un-exports everything.
I have questions.
* Version checking: clean up code, add server flags
FB-1975
Cleaned up version checking, removed a race condition, and added error checking.
Added server flags for check-in endpoint and UUID storage file.
Incorporates Travis's changes to store UUID file in data directory
and unexport most of verchk.go.
---------
Co-authored-by: Travis Turner <travis@molecula.com>
Co-authored-by: seebs <seebs@molecula.com>
When we close a task pool, we use a condition variable to wait for
workers to exit, if any workers are still running. The workers,
in turn, use the condition variable to notify that they've exited.
Unfortunately, the workers aren't using the lock (the rationale was
that it's all atomic ops so they don't need to), which means that
it's possible to have the following sequence:
Close(): obtain current live count
worker: decrement live count
worker: send broadcast to condition variable
Close(): wait on condition variable
To resolve this, we make the worker update also request the lock.
We add a simple reproducer for this. Note that simple doesn't mean
it fires completely reliably; on my laptop, the test causes a test
timeout about 60% of the time without the fix. If you add a short
delay between sampling the live count and waiting on the condition
variable, the deadlocks move from "60% chance of hitting it in
a million trials" to "nearly always".
* Add viper (for env variable) support to CLI
* Move the "featurebase cli" sub-command to its own "fbsql" command
I don't know if this is the final name, but putting it here as a
placeholder for now.
* Handle single `--command` flags.
This also adds a printer interface so we can opt NOT to print setup
information in non-interactive mode.
* Add support for multiple `--command` flags in the same call
* Add support for multiple `--file` flags
* Move members related to Config into a separate struct
* Make sure non-interactive mode can connect to a database
* comment fix
* support control-C on readline
* Prevent connection message from printing in non-interactive mode
* Return errors (instead of printing them) in non-interactive mode
* make nodeid come from the correct table
* refactored aggregates; added ability to aggregate on expressions not just references
* addressed feedback
* now with the compiler errors fixed after rebase
If we don't set an epoch, we get a cryptic message on the console.
Note, this message isn't logged properly, it doesn't use the
logger, it uses the `log` package.
2023/02/09 11:07:23 ERROR: converting timestamp options for
end_time: checking overflow: custom epoch too far from
Unix epoch: 0001-01-01 00:00:00 +0000 UTC
Because this uses the log package, it doesn't go to the same place
as other messages, making it a pain to debug.
The underlying problem is that a timestamp can't just have a zero
value for its epoch. So, we set a default epoch of 0 Unix Time.
We should possibly revisit the question of whether the conversion
in the top-level schema.go should handle an epoch which IsZero,
but I'm not sure what "base" should be in that case. In practice,
all existing usages except this one are specifying time.Unix(0, 0)
already.
Note: We now skip a test because we can't pass it but fixing
it is presently beyond my understanding. In parser_test.go,
we skip
SELECT * FROM X INNER JOIN Y ON true INNER JOIN Z ON false
because if we stringify it, we put the "ON true" in the wrong
place, and end up with something we can't parse.
The main change here is modifying AssertParseStatement and
AssertStatementStringer (and the corresponding Expression
functions) to also verify that they can clone and round-trip,
and that we can walk expressions. This gives us a ton
more coverage of Clone and conversions to string, and caught
a number of subtle typos and missing type switch cases.
Some of the changes are mostly cosmetic, such as only
including optional words when stringifying expressions
or statements if those optional words were present originally,
as shown by the Pos value stored for those words.
We also drop a lot of trailing apostrophes from some of the
parse test cases, which appear to be harmless but won't be
reproduced when converting back to strings.
We also add a number of additional test cases, or add
clauses to existing test cases, to improve coverage of a
lot of error testing. For instance, we added a decimal
field to the tests of show table, and added cases
using KEYPARTITIONS. (Although it doesn't *do* anything.)
Similarly, whenever we create a statement, we check
the behavior of requesting a list of sources from it, to
verify that source finding code at least runs.
* 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
* fb-1940 re-implemented some changes that got missed private-public
* fb-1939 fixes to between + decimals
* fb-1935 - avg() on and id type + fixed some tests
* fb-1953 add min/max for string types
* fb-1938 - remove internal_type column from show columns
* fb-1964 - fix space_used in fb_cluster_nodes to be int
* fb-1996 - make sure all Idents that are being used as object references to schema objects are lowercased
* fixed failing test
* added some missed changes
* fb-1969 found another case issue with identifier used for column idents
* 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
* 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
* 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>
* 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>
* only keep the last 2000 requests and truncate sql and plan text to 4K each
* fixed import
* address review feedback
Co-authored-by: Fletcher Haynes <fletcher.haynes@generalassemb.ly>
* first cut at pre-sort command that works on ndjson
* finish pre_sort command for CSV and JSON and add test
* try fixing golangci-lint
* remove some dumb lint checks
* more linter disabling
* take .golangci.yml from previous repo
* go fmt (facepalm)
* remove ioutil to fix lint
When we added a line feed to the user input, we broke the check for
"exit" (because after that change we were getting "exit\n". This commit
moves the exit check before the line feed append.
* Initial commit of code to do a version check-in on startup
* Add json tag to the response struct for version check
* Adjusted version check response types
* Changed error message in version check-in goroutine to use the logger. Changed URL to prod from dev.
* Updated version checkin URL to be analytics
Co-authored-by: Fletcher Haynes <fletcher.haynes@featurebase.com>
(cherry picked from commit c38210bef5)
* adding kafka consumer config options (--kafka-max-poll-interval, --kafka-session-timeout, --kafka-group-instance-id, --kafka-socket-keepalive-enable, and --consumer-close-timeout)
* wrapping consumer.Close() in timeout. Will wait consumer-close-timeout seconds before forcing consumer to exit
* clean up logs
(cherry picked from commit 17cdc58d80)
This commit moves the cli out of the `ctl` package and into its own
`cli` package. It also adds some basic tests for expected input.
Finally, it fixes a bug which was causing intentional line feeds to be
ignored, which was a problem with the BULK INSERT command.
(cherry picked from commit cf72bfa16f)
* 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)
* implement CREATE/ALTER/DROP VIEW
* fixed failing test
* another failing test
* fixed some broken serverless tests
(cherry picked from commit c620aae350)
* performance counters
* first cut of perf counters and system table fanout and a wire protocol
* significantly refactored prometheus support; removed statsd and exprvar
* removed node_id
* put dax subquery test back
* Change Translator.TranslateFieldIDs method to take a dax.TableKeyer
There are a bunch of other calls to the Translator interface methods
with currently take an `index string`, and those need to be converted to
dax.TableKeyer as well. But I need to review each call, because in at
least one place I noticed one being called with `result.Index` instead
of with the qtbl available. And I don't yet know how those could be
different.
Co-authored-by: Travis Turner <travis@molecula.com>
(cherry picked from commit 7f6ea0e6e5)
* handle sum aggregates with ints; handle escaped quotes in blob literals
* added test ceoverage
* skip subquery test for dax
(cherry picked from commit af475a27f2)
* Change JSON response name from exec_time to execution-time
Execution time stopped working in the CLI because it uses the latest
json tag.
* Wait, don't break the interface.
* Add a test for the sql response json tags.
This is to make sure that if someone like Travis just goes and changes a
tag name to be more consistent, that we perhaps catch that before it
gets to the end user.
* Change exec_time to execution-time after all
(cherry picked from commit b5dd3ea02e)
* implemented distinct
* implemented distinct
* uses first cut of a buffer pool, and extendible hashing with thresholded spill to disk
* tests
* cleaned up some stuff around query plan output to make developing tooling easier
* added optimization to call PQL Distinct()
* fixed test
* fix for passing wrong index name in orchestrator
* back out change to DistinctTimestamp
* fix other instance of wrong table name being passed
* use full index name instead of abbreviated one for translation. sigh.
* removed some unused code
Co-authored-by: Matthew Jaffee <jaffee@pilosa.com>
(cherry picked from commit f030d58d95)
- 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)
* return 200 once plan compilation starts; if error, return error in response.
* removed some commented out code that is definitely not needed.
(cherry picked from commit 9dda3ff215)
* expose Transaction on TranslateStore for DAX Snapshotting
* try to fix ramdisk nonsense
apparently, we were running in either a shell env or docker env
randomly, so this could sometimes pass and sometimes fail since the
shell env had the ramdisk set up and docker didn't.
Now we force to run in docker always and set up ramdisk explicitly.
* debug ramdisk issue?
* fix tests... and a buncha other stuff
The executor test I modified failed when I changed DefaultPartitionN
to 8, but just because stuff was out of order so I made it more
robust.
I edited some data gen stuff to make shorter lines because it was
making grep results unusable.
the actual fix is in translate_boltdb_test.go
* clean up, fix code review feedback
(cherry picked from commit 3693b9950a)
* delete implementation with test coverage
* optimize IN expressions; stop linter complaining
* fixed some uncovered query cases
* skip test in DAX for now
(cherry picked from commit 021219935f)
now that min/max queries don't use a write Tx it seems we need to
separate read and write in the tests. Not sure I 100% understand this.
(cherry picked from commit a58299def4)
CLOUD-1252
Implemented Jaffee's fix of checking for b.useShardTransactionalEndpoint
and only running the start/finish transaction block if it's false. Moved
stats timing to a separate defer so it could stay out of the if.
(cherry picked from commit b1f5264a4b)
* Fix PQL distinct in dax
When issuing a PQL Distinct() call (or any other call with a "index=" arg),
this commit will attempt to convert the value in the index arg with a
TableKeyer.
* Apply change to call.Children as well
* Add some PQL Distinct (join) test coverage
(cherry picked from commit 4e8fe488de)
- 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)
* Prevent file corruption when writing tar backup to stdout
FB-1794
Tar backups written to stdout were coming out corrupt. This turned
out to be due to log messages getting written to stdout and ending
up in the tar file. We now check to see if the tar file and the log
are both going to stdout, and if they are, send the logs to stderr
instead.
Testing did not have any kind of consistency or validity check. We
now compare a tar file sent to a file and a tar file sent to stdout
to make sure they're the same. This does not guarantee correctness
but does at least catch this form of corruption.
* trying different index name
Co-authored-by: tgruben <tgruben@gmail.com>
Co-authored-by: Todd Gruben <todd@molecula.com>
(cherry picked from commit a8996a149d)
* Make interfaces more specific than "MDS"
- Introduce `dax.Schemar` interface
- Introduce `dax.Noder` interface
- The rest is generally to standardize on the new interfaces.
- Remove `pilosa.SchemaInfoAPI` interface
- Move `TranslateNode` and `ComputeNode` types from controller to dax package
- Remove `queryer.FeatureBaseImporter`
- Remove `queryer.MDS` interface
- Remove `queryer.Importer` interface
- Identify types using an "MDS" interface and split into Noder/Schemar as necessary
- Changed `Queryer.orchestrator` to a `map[qual]*qualifiedOrchestrator` because we can't share an orchestrator across quals
* Convert orchestrator to use TableKeyer
(cherry picked from commit 14f1930004)
* Fix "qualifer" misspellings
* Remove `track_existence` and `shard_width` from SHOW TABLES output
* Thread Owner, UpdatedAt, UpdatedBy through SchemaAPI
I took the liberty of renaming "LastUpdatedUser" to "UpdateBy" to align
with "UpdatedAt".
(cherry picked from commit 63cfdb5078)
This commit introduces an interface called `TableKeyer` which anything that means to represent a "table"
can implement. Examples are `dax.QualifiedTable`, `dax.Table`, and `string` (for legacy pilosa calls
where Execute simply took `index string`).
In the case of `orchestrator.Execute()` and `qualifiedOrchestrator.Execute()`, we are intentionally strict
about which type of `TableKeyer` the respective method accepts. If we find, in the future, this is too
restrictive, we can loosen that; but for now it helps us understand what is expected.
(cherry picked from commit a61d1a9571)
* Move batch.Importer interface to pilosa.Importer
In addition to moving the interface, it updates all the methods to use
dax.TableID (for example) intead of a string pilosa index name.
* Change unused onPremImporter methods to no-op.
onPremImporter is a wrapper around API which implements the Importer
interface. This is currently only used by sql3 running locally in standard
(i.e not "serverless") mode. Because sql3 always sets
`useShardTransactionalEndpoint = true`, There are several methods which this
implemtation of the Importer interface does not use, and therefore they
intentionally no-op.
(cherry picked from commit 12d608c80d)
* Thread Table.Description through SchemaAPI
* Thread Table.CreatedAt through SchemaAPI
* Thread Field.CreatedAt through SchemaAPI
(cherry picked from commit 734477aaee)
* This adds in support to the lattice UI application to use the SQL3
endpoint. If the `/sql` endpoint returns 404, it will use the SQL1
endpoint. If the `/sql` endpoint is available, it will send SQL queries
to that. It does not try the SQL1 endpoint if the SQL3 endpoint returns
an error processing the query. That is, it is all SQL3 or SQL1.
- Below are the specific changes:
- Adds a file that contains functions for interacting with http services as opposed to just grpc/event-based services. As of this commit, it is only the SQL3 endpoint.
- This adds a variable to track if we are using the SQL3 endpoint or not
- This adds a function to handle the response from the SQL3 endpoint
- Adds a function to eventServices to query the sql3 HTTP endpoint
- Fixed a missing semicolon in grpcServices
Co-authored-by: Fletcher Haynes <fletcher.haynes@generalassemb.ly>
(cherry picked from commit 7ab453e289)
* WIP: Convert SchemaAPI to be DAX-centric
* Tables(), CreateField()
* CreateTable(), DeleteTable(), DeleteField()
* More cleanup
* Remove the old SchemaAPI
(cherry picked from commit a15783cb49)
* Move batch.Importer interface to pilosa.Importer
In addition to moving the interface, it updates all the methods to use
dax.TableID (for example) intead of a string pilosa index name.
* Change unused onPremImporter methods to no-op.
onPremImporter is a wrapper around API which implements the Importer
interface. This is currently only used by sql3 running locally in standard
(i.e not "serverless") mode. Because sql3 always sets
`useShardTransactionalEndpoint = true`, There are several methods which this
implemtation of the Importer interface does not use, and therefore they
intentionally no-op.
(cherry picked from commit 12d608c80d)
* Thread Table.Description through SchemaAPI
* Thread Table.CreatedAt through SchemaAPI
* Thread Field.CreatedAt through SchemaAPI
(cherry picked from commit 734477aaee)
* This adds in support to the lattice UI application to use the SQL3
endpoint. If the `/sql` endpoint returns 404, it will use the SQL1
endpoint. If the `/sql` endpoint is available, it will send SQL queries
to that. It does not try the SQL1 endpoint if the SQL3 endpoint returns
an error processing the query. That is, it is all SQL3 or SQL1.
- Below are the specific changes:
- Adds a file that contains functions for interacting with http services as opposed to just grpc/event-based services. As of this commit, it is only the SQL3 endpoint.
- This adds a variable to track if we are using the SQL3 endpoint or not
- This adds a function to handle the response from the SQL3 endpoint
- Adds a function to eventServices to query the sql3 HTTP endpoint
- Fixed a missing semicolon in grpcServices
Co-authored-by: Fletcher Haynes <fletcher.haynes@generalassemb.ly>
(cherry picked from commit 7ab453e289)
* WIP: Convert SchemaAPI to be DAX-centric
* Tables(), CreateField()
* CreateTable(), DeleteTable(), DeleteField()
* More cleanup
* Remove the old SchemaAPI
(cherry picked from commit a15783cb49)
* FB-1739: Add ability to add a description to a table on creation
- Added CommentOption to handle text after COMMENT option.
- added description field in the createtable plan.
- The description is stored in the existing index metadata.
(cherry picked from commit ad350c2d49)
* Clean up dax service interfaces
Rename some of the `computer` interfaces and organize them in the
appropriate files.
Remove `dax/computer/alpha` package
* Remove ComputeAPI (it was replaced by batch.Importer)
* add nss-tools dependecy to smoke test
(cherry picked from commit 969bf055b2)
* moved the debug code to the right spot
* enforce int min/max constraints on inserts
* add a check for decimal min and max
* fixed borked tests
* fix the decimal to int conversion in constraint check
Co-authored-by: Travis Turner <travis@molecula.com>
(cherry picked from commit e392ce3460)
* Introduce ServiceManager and Refactor DAX Integration tests
The ServiceManager provides an interface with which to manage
featurebase (dax) services (mds, queryer, computer). It replaces the
confusing interface implementations in /dax/server/server.go (which
optionally used pointers to in-process objects to satisfy an interface)
with (for now) http implementations. The thought is that even if we're
running all services in-process, we should communicate between services
over http in order to mirror what we would do in a production
environment where the services are running on different nodes.
This batch of commits does quit a lot, most of which is captured here:
- Added `path` support to `dax.Address`. Address is now a string of the form [scheme]://[host]:[port]/[path].
- Added `Holder.directiveApplied` to determine (in tests) if the computer has completed applying the latest directive. This is somewhat temporary until we improve the mds-to-computer logic.
- Removed the "service prefix" code which was prepending client URL paths with the prefix. Instead, the serviceType (mds, queryer, computer[n] is now part of `dax.Address`).
- Removed, from the dax config, the top level `StorageMethod` and `StorageDSN` and now just have `MDS.Config.DataDir`.
- Added `Computer.Config.N` to specify the number of computers to run in-process.
- Moved the `pilosa.MDS` interface to `computer.Registrar`. This is an example of getting the interfaces defined in the right packages.
- Added `SnapshotTable()` method to the mds client (to align with its API).
- Changed `Balancer.AddJob()` to `Balancer.AddJobs()` to support, for example, adding 256 partitions in a single call. Refactored some of the naive Balancer to account for this.
- Added a `Seed` to the top-level config. It's not really useful because of package `crypto/rand`.
- Added an in-memory implementation of the DisCo interface and disabled etcd in a computer service.
- Create sepearte data-dirs for each in-process computer.
- Disabled grpc in dax.
- Modified the sql3 test definition format to support multiple insert steps and separate query results (to align with those steps).
* Changes necessary to get multiple computer instance running in-process
For now the config looks like this:
```
[computer]
run = true
n = 4
```
but we can probably just change that to be something like:
```
[computer]
run = 4
```
*Issues found running multiple "computers" in-process*
- grpc was trying to bind on the same port
- changed GRPCListener from `*net.TCPListener` to `net.Listener`
- created a nopListener and set to that for now (i.e. disabled grpc)
- etcd was starting more than once
- changed dax to use in-memory implementations of the disco interfaces (i.e. stop using etcd)
- IDAllocator (which uses boltdb) was trying to open the `idalloc.db` file more than once
- realized we have to set separate data-dirs for each holder. that fixed it.
* Port dax integration tests to ManagedCommand
* Modify Balancer-related methods like AddJob to AddJobs
There were (and still are) a lot of places where we were adding on job
at a time, even when we had a long list of jobs to add. This resulted in
every job add (for example adding 1 of 256 shards) taking ~40ms, or over
10s to create a keyed table. One reason was because each job add was
making multiple boltdb transactions.
* Port over more dax integration test stuff
* Add DirectiveApplied to signify that snapshot/writes have loaded.
We use this in tests to avoid using sleeps.
This should be considered temporary; we're going to need a more robust
solution for determining when a computer node is ready to serve complete
data.
* Finish porting dax integration tests
* Improve godocs
* Remove docker-based DAX integration tests.
* go mod tidy
* Move test/managed.go to avoid package conflicts
* Modify IDK integration tests to work with ServiceManager changes
This is really just computer -> computer0
And the MDS DataDir config change.
* cleanup found during review
* echo $CI_COMMIT_REF_SLUG in CI
* remove docker image arg, use build instead
(cherry picked from commit 2843f218bc)
* fixed a bunch of issues with non-pql aggregation; moved some decimal related functionality; made top actually top (for the non-pql case); experimental create function
* drive up test coverage
(cherry picked from commit 0be0c42b66)
implements an fb_exec_requests system table. The purpose of this table is to allow access to internal state to see what queries are running and have been run.
Co-authored-by: Travis Turner <travis@molecula.com>
(cherry picked from commit 47d8be26f5)
Reproducible builds are something we should be doing, and we are there
as far as making them in CI is concerned with this change.
The changes to the Dockerfile/Makefile do nothing if the
SOURCE_DATE_EPOCH environment variable is not set before `make build`
happens, or if the build arg is not passed in to docker.
(cherry picked from commit fd93fc99c9)
* updated optimizer to be smarter when trying to push a top operator down; added test coverage
* skip a dax sql test that keeps failing
(cherry picked from commit e0d6b292bc)
* fixed error messages for alter table add and drop; added test coverage
* Removed two CI tests that are failing intermittently for no known reason.
(cherry picked from commit 3c2c8c6011)
FB-1771
In api_test.go, it was being used to make sure that incorrect input
produced the right errors; this is now handled by making sure the
error isn't nil and then checking its string against the expected
error string.
In executor_test.go and internal_client_test.go, it was being used
to compare QueryResponse structures, which contain an error.
handler.go now has a function specifically for comparing them,
which can provide additional detail if necessary.
Added a test for SameAs to handler_test.go.
(cherry picked from commit 775fd0b08c)
* tighten up checks for order by expressions fixed ordering by expressions
* added testing to cover order by cases
* Add DecimalAgg member to proto GroupCount definition
In DAX, where we have split the orchestrator from the executor, and the
orchestrator can run on a different host, there are cases where
`GroupCount`s can travel over the wire via the Internal Client. In these
cases, when the group count contains a decimal aggregate, we need to
send that value as the appropriate type.
* fixed missing cases in order by and case block eval
Co-authored-by: Travis Turner <travis@molecula.com>
(cherry picked from commit 158cc669d9)
stdout/stderr around
A lot of functions in the cmd and ctl packages were passing these
around and barely using them. Replaced them with a logger for most
functions. Some functions get an io.Writer instead so that their
tests can find the output they're looking for.
More cleanup on fb-1766: reworked the tests that were using io.Pipe
or os.Pipe to check their results so they now use a bytes.Buffer.
Unexported some variables that didn't need to be exported.
Fixed NewConfigCommand to use the provided stderr, not os.Stderr.
Added tests for rbf_dump, rbf_page, and keygen, since those weren't
being tested at all.
Added chksum_test, final cleanup.
(cherry picked from commit f627199acb)
There are now only four tests remaining which do not pass.
One is related to error format mismatch.
Two require orchestrator work.
One won't pass until table name conversion is supported for multiple
tables.
(cherry picked from commit 5bf5b5364d)
MarshalLogMessage serializes the log message and prepends additional encoding
information to each message. Currently, we prepend three bytes to each log
message:
byte[0]: encodeVersion - this is currently a constant within the code. If we
modify structs such that they encode differently, we'll have to change the
constant and keep previous versions of structs for deserialization.
byte[1]: encodeType (e.g. "json", etc.)
byte[2]: logMessageType
If we get into a situation where we want more flexibility in these message
header bytes—for example, if we want to use more than three bytes—we could do
something with the first bit of the encodeVersion: if it's 1, that could
indicate that there are additional header bytes, and the following seven bits
could indicate how many.
(cherry picked from commit 6740bc250e)
* Fix formatting in CLI results with custom SQLResonse.UnmarshalJSON
When I started this, it was meant to be a quick fix to address the confusing
result formats we were seeing in the CLI. For example, all large integer values
were displayed in scientifc notation. This is because we were passing the result
types from JSON (in this case, float64) into pretty print. Similarly, `IDSets`
and `StringSets` where being printed using the default go Stringer for the types
[]int64 and []string respectively.
I started by writing a customer UnmarshalJSON() method for the `SQLResponse`
type. Part of this (the part which converts data types based on header types)
was already being used in dax tests, so this just formalizes that logic as part
of the `SQLResponse` type.
Then I realized that the sql3 tests (run against the `sql3` package) were
failing because sql3 is not actually returning the `IDSets` and `StringSets`
types. A future task is to formalize return types, define them, and modify sql3
to return them. Once that is done, we can remove the "typed" switch in the
`SQLResponse` json unmarshaller.
Another significant change is the modification to the `ExprDataType` interface:
```
type ExprDataType interface {
exprDataType()
TypeName() string
TypeDescription() string
TypeInfo() map[string]interface{}
}
```
I added two more methods in order to distinguish between a type (`DECIMAL`), its
description (`DECIMAL(2)`), and its type info (`"scale": int64(2)`). Currently,
the description can be used as the field definition in a CREATE TABLE statement,
but we may want to re-think that. Also, Decimal is the only type currently using
TypeInfo.
Finally, I tried to consilidate things around `dax.FieldType` instead of
comparing against parser types outside of sql3. We still have some sql3 parser
and planner types lurking about, but we can address those in future commits.
* Add some test coverage
* smoke test expected INT, now int
* minor fixes
* Introduce WireQueryResponse and related types
This also changes dax.FieldType to dax.BaseType.
* Populate WireQueryResponse correctly
Currently this is in the http handler, and in the queryer.
* Convert sql3 and dax tests to expect pilosa.WireQueryField in results
* fix PQL tests in the SQL defs
* Address a few of the skipped sql tests in dax
(cherry picked from commit f4385df2cf)
this commit changes the way the plan is retrieved; implements Stringer on types.PlanExpression in preparation for HAVING support; removes last vestiges internal float64 arithmetic; implements a filter on PlanOpFilter; fixes various bugs in the PlanOptimizer when rewriting qualified references
* fixed selects with unqualified identifiers
* handle bad and non-existent query param inputs more appropriately
* added test coverage for PlanExpression Stringer
Co-authored-by: Matthew Jaffee <jaffee@pilosa.com>
(cherry picked from commit 5f662d2bce)
We use part of the dax.TableName in the pilosa.Index.Name.
This just ensure that we don't let invalid characters get through.
(cherry picked from commit 10e8aa5c45)
the BackupTar and RestoreTar functionality was ignoring provided
readers, which doesn't matter for real usage but breaks tests
by making them dump raw tar binaries to stdout.
(cherry picked from commit 93153a97db)
Cobra automatically displays usage messages, and also a gratuitous
"Error: [...]" line in some cases, when any error at all occurs
running a command. To suppress the usage message, you have to set
cmd.SilenceUsage to true. But the code that would do this doesn't
have access to it. To address this, we introduce a category of
"usage error", implemented with stdlib error wrapping (%w) and
use errors.Is to check for it. There's also utility functions
to do this checking automatically, or indeed, to handle wrapping
of the ctl.SomethingCommand and handle running it with a suitable
context and everything.
In fact, several of the places we're checking for usage errors,
we can never actually report one, but we're checking consistently
so that if we want to report usage errors, we can.
For instance, server.Start and (dax)server.Start don't ever
return usage errors, right now, but we're checking their responses
anyway.
(cherry picked from commit c681642734)
The idea behind this is to give AWS more information about what
instances we can let it actually instantitate, rather than have it be
one fixed instance type.
e.g., in this case, we are okay with any Graviton instance with at least
2 vCPU and 8 GiB memory.
The easist way to do that is to instead use a launch template, with
ec2_fleets or spot fleets.
I took out the part where we even support on-demand instances. This can
be readded later if it is necessary.
(cherry picked from commit 3bfb65eb58)
* Rename dax.Shard to dax.VersionedShard
* Rename dax.Partition to dax.VersionedPartition
* Rename dax.FieldVersion to dax.VersionedField
* Rename go files to a standard
(cherry picked from commit 49d0e0fbc8)
This gets complicated. For coverage output, sonarcloud supports wildcards.
For test output, it doesn't. So we weren't getting meaningful results,
just weird error messages. I fixed that, and got thousands of lines of
other error messages because it wasn't finding the test source files.
That looked like this:
WARN: Failed to find test file for package
github.com/molecula/featurebase/v3 and test
TestTranslation_Primary
But we don't actually need the test reports sent to SonarCloud, because
"which parts of your test suite are being run" is sort of inherently
"basically all of them" with go test. So rather than continuing to do
that, we drop it.
Since we're dropping that, we don't need the JSON output from go test
anymore, so we drop that too, and the tee commands, and the "artifacts"
from the tee commands, and now our test output is human-readable and
slightly faster.
We also bump SonarCloud to 4.7.
We also fix the tests to use GOVERSION sometimes and GOFUTURE other
times, and bump from 1.19.2 to 1.19.3.
Also a couple of minor cleanup (adding explanatory comments,
combining adjacent grep commands, etc.)
(cherry picked from commit da4fb4ab4e)
Testing unicode is great, but we appear to have had a couple
of cases where we were using strings that weren't valid UTF8.
Weirdly, other instances of these strings work -- I think because
they're in raw quotes (backticks) rather than strings. Anyway,
this is what SonarCloud fusses about.
(cherry picked from commit ab543adbe3)
In this commit, the Directive is mocked; it doesn't actually reach out
to a controller.
Limits key translation to only those partitions (per index) specified in
the Directive. Attempting to create or find a key (or ID) for a
partition which is not handled by this node will result in an error;
translation requests are no longer forwarded to other nodes.
Limits import into only those shards specified, per index, by the
Directive. Attempting to import into a shard which is not handled by
this node will result in an error.
Stub out /directive endpoint
The `applyDirective()` method still needs to be implemented.
Update mds references to use the new /mds/types structure
In mds, we moved the shared types to mds/types. FeatureBase needs to
reference those instead.
This also bumps the mds version in go.mod.
Implement the Add/Remove Index part of Holder.ApplyDirective()
This adds functionality to `Holder.ApplyDirective()` which adds or
removes indexes (tables) based on those provided in the Directive. Still
to be implemented here: shards and partitions.
WIP: remove client from Batch
Move Batch into its own package: batch
Also, in order to avoid import loops, this introduces packages:
/batch/types
/client/types
Reorganize the Importer-related code
Moved the Importer interface to package: batch
Move the "pilosa client" implementation of the Importer interface to
package: client
Modify batch.NewBatch to take an Importer (not client)
This commit modifies the batch.NewBatch() function to use a functinal
option on Batch to inject an Importer into the Batch. Prior to this,
NewBatch() took a pointer to a client, which was a little too
restrictive. Now, MDS can implement an Importer which uses information
from MDS to determine to which node(s) the import calls should be directed.
Add client.SetAuthToken() method to satisfy SchemaManager interface
Update ApplyDirective logic to include fields.
This needs more work, but it was enough to get a basic test passing.
Move Transaction type into /types package.
Add interface check on batch.Importer no-op implementation
Updated ApplyDirective to create all currently support Field types
There are still the following TODOs:
- [ ] impolement field options (ex: decimal scale, int min/max, etc).
- [ ] `time` fields
Added support for Decimal.Scale in ApplyDirective
Update mds dependency
Add /health endpoint
Update to use dax (dax/mds) instead of mds.
After moving the mds repository into the dax repository as a
sub-package, this commit changes everything in FeatureBase to use the
dax repo instead of the now abandoned mds repo.
Introduce and implment the WriteLogger interfaces.
This adds both a `WriteLogReader` and `WriteLogWriter` interface. They
are both implemented by the implementation: `fileWriteLogger`. The
`fileWriteLogger` uses the dax/writelogger API to append log messages to
files on disk.
Add WriteLogWriter.ImportRoaring method to interface
This commit adds the `ImportRoaring` method to the `WriteLogWriter`
interface. Still to implement are the `Import` and `ImportValue`
methods.
Reorganize the ApplyDirective code
The primary goal was to cache the incoming Directive on the Holder prior
to applying all of the changes in the directive (i.e. loading data from
the WriteLogger) because applying those changes often validated against
the accepted state of the node.
Implement all of the WriteLogger read/write methods
Implement the HTTP WriteLogger implementation
WIP: Introduce shard.Version. Implement snapshotter.
Add HTTP Snapshotter implementation
This also recofigures server to use the HTTPSnapshotter instead of the
FileSnapshotter.
Implement snapshotter: TableKeys
Implement snapshotter: FieldKeys
Dependency dance
last of the dependency dance
Add support for prototype
This adds the Makefile targets to build the docker container and push it
to ECR.
SQL3 changes which break with dax changes
Missed TODO: implement FieldVersion version to WriteLogger
Address bug causing missing TranslateStores to error
Originally, we tried to limit the TranslateStores which get allocated to
only those for which the node is responsible. This works when adding a
new table. But if a table already exists, there's no logic to start
missing TranslateStores.
This reverts back to the old FeatureBase logic which brutishly allocates
a TranslateStore for every partition, even if one is not needed.
We need to address this by allowing the ApplyDirective logic to
initialize TranslateStores when they don't yet exist.
Move the ImportRoaringShardRequest type to the types package
Since the ImportRoaringShardRequest object is part of the Importer
interface, we need to move it to a non-root (i.e. pilosa) package. All
the other interface types are either concrete types or part of a
sub-package (such as roaring). We do this to prevent an implementer of
the interface from having to import the entire pilosa package and risk
circular imports.
buncha changes to support latest dax stuff
Move dax related types to /dax sub-package
This commit moves all the common "dax" types into the /dax sub-package.
The idea is to ensure that featurebase does not import dax at all.
It's ok if dax imports featurebase.
In the future, we might need to split the dax sub-package (common data
types used by muliple molecula data-plan services) into it's own repo.
Add type: dax.Schema
This isn't currently being used; I started to use is as a replacement
for pilosa_client.Schema, but then deferred that. But we'll need to do
it eventually, so it doesn't hurt to have this type in place.
Export RowIDs.Merge() method for use in orchestrator.
Add CreateSQL method to dax.Table type
The CreateSQL() method will return the "CREATE TABLE" statement required
to create the dax.Table.
Comment out confusing writelogger log message.
We need to revisit this, but for now, this log message is confusing.
Also, rename daxSharder to versionStore.
Remove hard-coded AWS account
Implement more FieldOptions such as Epoch
Some of the FieldOption logic was stubbed out in the dax package. This
commit fills that out more; specifically, it adds the
dax.Field.Options.Epoch parameter.
export stuff needed for TopK in orchestrator
export ValCount stuff to implement Percentile in orchestrator
export more stuff to support less code in orchestrator, shared objs
Port dax repo over to featurebase/dax (run all as sub-services)
This commit does ALOT. Sorry.
It introduces a `featurebase dax` sub-command which can be configured to
run the various dax services as sub-services within the same process, or
individually as the lone service in process.
It also changes all the URL paths to be prefixed with the service name.
So for example, instead of calling localhost:8080/status, you would now
call localhost:8080/featurebase/status.
Also, note that all services provide a /health endpoint to confirm they
are running in process.
Clean up integration tests. Remove PILOSA_ config prefix.
Remove duplicate clients (mistake from porting dax to featurebase)
Rename sub-service "featurebase" to "computer"
In the places where we have hard-coded the sub-service name into a URI
path, I've tried to tag the line with a comment containing:
`// #SERVICEPATHPREFIX`
Update copilot manifest files to reference "computer"
Port dax/README.md from dax repository
Separate (toml) Queryer Config from Injections
We needed to separate the toml config from the configuration required to
inject sub-services into the Queryer. I'm not sure this is the best
solution, but it's *a* solution. So here we are.
Clean up (i.e. remove) the queryer "implementations" package
Remove old test file
Run WriteLogger and Snapshotter as local sub-services.
Prior to this commit, the writelogger and snapshotter services only
worked when run as separate services. This allows them to be run in the
same process as all the other dax services.
There is still some naming issues that we should address, but it's
functional for now.
Clean up (i.e. organize) the intra-service interfaces.
Implement alpha Director for local messages from MDS to Computer
Prior to this commit, messages from MDS to the computer service were
still going over http. This commit introduces an interface
implementation which registers the local computer command, and use that
command's API to directly reference methods used by the Director.
Clean up a few more interface names
Add Queryer OpenAPI document.
Update copilot manifests to reflect latest changes
Add OpenAPI documents for WriteLogger and Snapshotter
Add OpenAPI document for MDS service
Add OpenAPI document for Computer service
Consolidate errors to use fb/errors package.
This commit is a first pass at trying to ensure that all of the DAX code
uses:
"github.com/molecula/featurebase/v3/errors"
This package is a wrapper for "github.com/pkg/errors", so going forward
we want to avoid importing that package.
The only method which isn't backward-compatible is `New()`; the
New() method in the featurebase/errors package takes an errors.Code. If
this becomes a problem, we could change this by reverting New() and then
introducing something like NewCoded(). But for now I think it might
actually discourage someone from just creating a New() error without
thinking about how it should be coded.
Introduce VersionStore interface
Move the existing VersionStore code to the `inmem` package as the
in-memory implementation of the new dax.VersionStore interface.
Introduce NodeService interface
With this, the Controller can maintain a registry of nodes by using this
NodeService interface as opposed to an in-memory map of nodes on the
Controller struct.
This also adds an inmem implementation of the NodeService interface.
Introduce controller.Balancer interface
This moves the existing balancer package to controller/naive package.
The idea is to allow us to add a different Balancer implementation in
the future.
Introduce DirectiveVersion interface
This commit also includes *A LOT* of refactoring to use dax.Worker and
dax.Job types everywhere instead of strings.
Introduce Schemar interface
The previous `Schemar` struct was moved to the `schemar/inmem` package,
and `Schemar` is now an interface implemented by that inmem package.
Remove unused type `nUnit`
Add boltdb implementation of VersionStore interface.
This removed the previous sqlite implementation; we decided not to use
sqlite for now (as a basic, local disk implementation) because it
requires CGO.
--------------------------------------------
No longer applicable:
Add sqlite implementation of VersionStore interface.
This commit implements the VersionStore interface using sqlite. Sqlite
requires CGO, so this may not be something we want to include, but it's
implemented here to get a feel for how an external implementation might
be used; the next step will be to determine how the user configured
FeatureBase to run using sqlite as a backing store for services like
MDS.
Add boltdb implementation of NodeService and DirectiveVersion interfaces.
Add boltdb implementation of naive Balancer interfaces.
This includes the two interfaces defined in `naive/balancer.go`:
- WorkerJobService
- FreeJobService
Add boltdb implementation of Schemar interface.
clean up a linter issue
Thread context.Context through all the interfaces.
Some of the interface implementations are going to use context, so we
need to make that part of the interface. The boltdb implementations, for
example, take a context. This is probably so we can do things like
cancel or timeout operations.
Update interfaces to return error; remove `panic(err)` everywhere.
Down-rev grpc version to 1.38.0
Later versions (after 1.42.0?) cause MustRunCluster.Close() in tests to
deadlock.
This commit also adds an `isComputeNode` feature flag around some of the
write log and shard/partition check functionality so that it doesn't run
under normal conditions (this is excercised by running the sql3 tests
for example).
Add MDS_Persistence test to cover meta data persistence
This adds a basic test which configures the MDS container to use boltdb
as its persistence storage, saved on a docker volume. Then, the mds
container is stopped/replaced, and we confirm that the data stored on
the volume is availble to the new MDS container.
Fix a few things after rebase with sql-experiment branch
The lastest version of sql-experiment contains a fairly significan
refactor of the way query iteration works. This commit adjusts for those
changes.
pull dax IDK changes in to FB IDK (#2177)
* pull dax IDK changes in to FB IDK
* Move docker-related IDK build stuff to featurebase root
Building the docker image required the root level go.mod and vendor
directory. This change moves the make targets to the root level
Makefile, and the Dockerfiles now copy the root level vendor directory
(and everything else in the root for that matter).
* Fix batch- and client-related tests
* InitializePoller on MDS restart/replacement
Prior to this change, if MDS was restarted, its internal poller (which
maintains an in-memory list of nodes to poll) is empty. This is bad,
because it doesn't know about nodes that it should be polling.
This change fixes that. Upon MDS startup, it intializes the poller with
the list of nodes that MDS keeps in persistent storage (currently:
boltdb).
* Add EFS volume to MDS Copilot manifest
This allows us to use MDS's persistent storage (via boltdb) in the
Copilot demo by saving metadata in a boltdb file on EFS.
* Thread logger.Logger through all dax components
* Revert some of the breaking changes from DAX development.
When we first started prototyping DAX, we made changes to the
featurebase core code which would have broken the existing featurebase
functionality. This commit reverts some of those changes. Anywhere that
we need to modify core featurebase functionilty, we put it behind some
kind of feature flag. This flag is typically determined by whether the
running node is a "compute" node (i.e. DAX.COMPUTER.RUN = true).
Co-authored-by: Travis Turner <travis@molecula.com>
add packaging for DAX
need cgo for datagen build
bind to 0.0.0.0, pass GOOS and GOARCH explicitly
not sure if the explicit GOOS/GOARCH is actually necessary...
Get INSERT INTO (aka ingest) working through SQL3
This commit does a few things which I'll try do describe here.
- Introduces a Qctx interface. The existing Qcx is an implementation of
this interface, and can be used exactly how it has been. But this
allows us to abstract away the notion of Qcx in the Queryer (which is
handling SQL3) until we're ready to address that. As an example, the
Qcx has a notion of a featurebase Holder, but that doesn't make sense
when we're at the Queryer layer. For now, the Qctx used in the Queryer
is a no-op.
- Adds a ComputeAPI interface implementation for the Queryer. This is
effectively the Import() and ImportValues() methods used for ingest.
The logic here handles the incoming ImportRequest by first doing any
necessary column and row translation for the entire request, then it
splits the records by shard, and generates a new ImportRequest per
shard with only the shard-appropriate records.
- Changes the mds.Importer to take an MDS interface implementation
(which can be an mds client) instead of an mdsAddress. This allows us
to use a localy MDS implementation rather than assuming we need a
client to make calls over a network.
Add queryer.Importer interface to handle ingest via SQL (#2203)
* Add queryer.Importer interface to handle ingest via SQL
This is meant to support ingest through SQL when the queryer and the
compute services are running in the same process, or when they are on
seperate processes and need to talk via http client.
* remove datagen from RPM
was originally added as a convenience to generate test data, but is
unused and annoying because datagen doesn't easily cross-compile due
to cgo
* add marshalUnmarshal to controller to avoid passing pointers
passing pointers across API boundaries can cause unpredictable things
in local vs remote configurations.
Co-authored-by: Matthew Jaffee <jaffee@pilosa.com>
"fix" a few issues with wrong default partition numbers
these still need to be properly fixed and actually get the correct
data from MDS
go mod tidy
Introduce TableQualifier (OrganizationID/DatabaseID) (#2220)
* add check in ApplyDirective that version is increasing
fix TestAPIDirective to make version always increasing
* fix docker image build and break out dax test in CI
We have to run the DAX integration tests separately as they call out
to Docker, and so it isn't easy to run them in a Docker container as
the other tests do. So we run them directly on the CI runner which has
Docker and Go installed.
We also explicitly exclude these tests from running during the other
tests.
Also my editor was automatically reformatting some comments badly
which is why I added the "data" thing in those two places
* add timeout to poller
* give Poller a default Logger
apparently we can NPE sometimes, seen in CI: https://gitlab.com/molecula/featurebase/-/jobs/3028286364
* bunch of testing fixes, mostly IDK/DAX related
make MDS error if sendDirectives errors, don't just
log. sendDirectives can error if computer nodes disagree about the
validity of a schema (for example), in which case it might need to get
deleted and user notified somehow. very messy, needs more thought.
re-introduce old env prefix to maintain compatibility with master
branch
make self-contained dax container for IDK testing
build IDK images from source (now that all the source is available
since it's in the same repo)
catch errors in DoExtractQuery in idktest.go
fix IDK bug where prefix path was hardcoded in all cases rather than
only when useMDS was true
fix TestBatchTargetMDS... needed to add field options and catch error
when creating table. also needed an _id field
* fix env prefix in tests
* WIP getting tests to pass, wanna see CI
* don't error if we get a zero version directive and we don't have a
directive yet
* cleanup debugging junk
* "fix" future.rename thing, run IDK tests
* Introduce TableQualifier (OrganizationID/DatabaseID)
This commit introduces a lot of new types (in dax/table.go) related to
TableQualifer (which is made up of OrganizationID and DatabaseID), as
well as things like TableID and TableKey.
For the most part, we try to thread a QualifiedTableID through the
entirety of DAX. There are some places (for example in the Balancers,
which are just aware of string keys) which use a string TableKey
(tbl__org__db__tableid).
* Remove some debugging comments
* Add Org/DB support to CLI.
This commit adds support for special commands:
SET
SET ORG acme
SET DB db1
USE db1
* remove ".pulled" from IDK Makefile
I don't think we need it any more as most things can be built
locally. I think it was only there to refresh the FeatureBase images
that were tagged as master, but we don't need to do that any more.
* Change DAX json tags to kebab-case (i.e. hyphenated)
This commit also renames some struct arguments to more accurately
reflect their type: for example, renaming `Table` to `TableKey` when the
type is TableKey.
* Return DAX TableName in SHOW TABLES (instead of Index.Name)
There are cases where SchemaAPI is used to return DAX friendly table
names (as opposed to featurebase index names, which are DAX TableKey).
This is an attempt to do that. With that said, it's not ideal because
anything could call those API methods and expect the other type.
* Fix a bug which wasn't completely dropping a table.
When using boltdb as a backend, DROP TABLE wasn't removing the
reverse-lookup key for the table in boltdb.
* Remove idk/testenv/certs which got accidentally committed.
also update .gitignore to include those.
* Fix IDK ingest tests to be TableQualifier aware.
* Add example Table types to dax/table.com godoc.
* ignore idk.Main fields for flags, upgrade commandeer
* go mod tidy
* Fix DAX integration tests: ingester using wrong ENV VARs
We change from ORGANIZATION_ID to ORG_ID
and from DATABASE_ID to DB_ID
* Clarify things around idk (docker) tests
* Stop running TestKafkaSourceIntegration with t.Parallel()
This test can't be run in parallel as it's currently written. Doing so
allows for interleaving of messages to the same kafka topic between
tests.
I didn't attempt to modify the test so it could be run in parallel. That
could be done, but left for someone more ambitious.
Co-authored-by: Matthew Jaffee <jaffee@pilosa.com>
Require Directive.Version be a non-zero value. (#2227)
Because the directive cached on the holder is not a pointer, its default
version is 0. In order to avoid having to compare against that, we just
require that Directive.Version start at 1.
General, non-invasive code cleanup and comment adjustment.
Move ImportRoaringShardRequest out of the types package
Early on in the DAX development, I moved ImportRoaringShardRequest into
a types package. There must have been some import loop going on, but
since that is not longer the case, it's safe to move this back into the
core featurebase (er... pilosa) package.
Move Transaction struct back into the pilosa package (from types)
Revert some name changes (cli -> client)
Add DAX Handler CloseTimeout
This was implemented in htt_handler.go, but it had been commented out in
the DAX handler. This just uncomments that and finishes the
implementation.
Remove Qcx from queryer.Importer interface
This sets us up to revert the Qctx interface that was initially
introduced to allow us to abstract away the need for a Qcx when calling
the ComputeAPI from a remote service (i.e. the queryer).
Add some go-doc comments and remove unused code.
Move SchemaManager setup from datagen to idk.Main (#2233)
The set for idk.SchemaManager (for dax implementations) was previously
in datagen. This may have been because of some import loop problem
during development, but that's no longer an issue.
The setup for this should be in idk.Main so anything using that can
leverage the MDS-specific SchemaManager setup.
Fix issues around nil TxFactory
First, don't return a nil. Rather return a new *TxFactory (with no
holder).
Second, don't call `f.holder` in the testhook outside of checking if
`f.holder` is nil.
Wrap all bare errors
Make service prefixes constants
Instead of having `"computer"` throughout the code, use instead a
constant: `dax.ServicePrefixComputer`.
MDS skip errors when sending empty directives
also add in the docker-login and ecr-push changes for serverless DAX
Fix the logic in Directive.IsEmpty() (#2236)
Update the cached value for Index.translatePartitions
In the case where a node already knows about an index, but its
assignment of partitions for that index changes (for example, when
another node goes down and the node in question is now responsible for
more partitions than it previously was), then we need to update the
cached value of Index.translatePartitions because that's used in
translation checks.
minor fixes for IDK-related bugs
WIP: tokenize CLI to access cloud
FB CLI cloud support with automatic token refresh
Also adds support for a GET command which allows making HTTP GET
queries to cloud CP which can be handy for debugging stuff. E.g. GET /v2/databases
buncha little fixes working on writelogger stuff
fix writelogger/snapshotter setup bugs
implement writelogging for importRoaringShard
add debug endpoint to MDS
use shard transactional endpoint in MDS datagen
add debugging to API related to writelogger
revert handleroption change
clean up big PR
remove "GET" command from CLI for making arbitrary HTTP request to
cloud control plane (was a messy hack and not that useful)
remove json tags from FB objects where we had to duplicate the object
elsewhere due to import loops and weren't actually json encoding it
unexport handlerOption which was exported to try to avoid doing
certain things if we're in DAX mode, but I didn't end up merging that code.
remove (hopefully) unecessary extra call to api.indexField
fix some formatting, unexport some vars, godoc, etc
oops, fix build failure
Update FeatureBase CLI to support a standard deployment
The standard deployment uses a different endpoint and request payload.
This commit tries to detect is the standard deployment is being used,
and if so, it uses a standard-specific FBQueryer.
It also modifies the auto-detection logic to try standard featurebase
and dax ports in the case where a port was not provided.
MDS API refactor (#2259)
* MDS API refactor
table IDs are exposed but only created server side
also cleaned up dax Makefile
* clean up review feedback
Co-authored-by: Travis Turner <travis@pilosa.com>
* remove TablesByName
* rip out inmem implementations and use boltdb everywhere
* remove inmem balancer, create bolt tempfile by default on startup
* WIP on snapshot table impl and test
* Minor comment and code layout adjustments.
This commit also adds the `Equals` method to `QualifiedTableID` for
equality comparisons. It's no longer safe to compare struct (two structs
might still be equal even if one of the structs doesn't have a `Name`
value.
* Use a unique docker network for each dax test
Ocassionally we would see some test failures due to a network already
existing. This shouldn't happen, but to avoid that, this commit
generates a unique name for each sub test (which gets deleted at the end
of every test).
* Fix one instance of NewQualifiedTableID losing Name
We should probably check the other instances and see if Name is getting
lost.
* simplify unique network stuff and fix api directive tests
* Remove TableIDRequest and TableIDResponse types for /table-id (#2267)
For the mds/table-id http requests, just use dax.QualifiedTableID as
both the request and response types.
* remove lattice from dax, no error on node re-reg, dax docker-compose
* various updates
* WIP: mds-refactor branch review
* no-op on SnapshotTableKeys if table is not keyed
* Makefile helpers
* add doWeCare so controller doesn't fail unnecessarily
* clean up table creation (#2272)
* Strip underscores from TableID stub name
* fix boltdb versionstore tests: generate unique, sorted tables
* fix controller test related to reregistering a node
* JobSet -> generic Set
Co-authored-by: Travis Turner <travis@pilosa.com>
Co-authored-by: Travis Turner <travis@molecula.com>
Cleanup after rebase on master
The latest rebase on master entailed all the client/batch changes as
well as some of the qcx refactoring. It made for a hairy rebase. This
commit fixes some of the tests that were failing after that rebase.
Fix batch/client import loop missed during rebase (#2280)
It's not surprising that `batch` can't import `client`. It was doing
that here (importing an error type from the `client` package). What is
surprising is that it's okay for `batch_test.go` to import `client` even
though `batch_test.go` is an internal test and therefore part of the
`batch` package.
different boltDB's for schemar/controller, explicit balancers
nice helpers for dax docker-compose, make build really fast
build FB binary outside of docker, then create Docker image with its
working dir in an empty subdirectory so it doesn't send a GB of
context to the daemon.
error on unassigned jobs and use client with timeout
fix CR feedback
deregister batch of nodes
also make removal faster via director dial timeout
implement WorkersForJobPrefix so orchestrator doesn't make up shards
also fix some godocs and remove unused method
Run sub-tasks of a Directive concurrently in a worker pool. (#2275)
* Run sub-tasks of a Directive concurrently in a worker pool.
This allows the compute node to concurrently load shapshot and writelog
data concurrently, instead of one keyset/partition/shard at a time.
It introduces a config parameter called `DirectiveWorkerPoolSize`.
* code review cleanup
* Use unique container names in DAX integration tests
We were seeing "container already exists" errors in CI, so just to be
safe, this commit constructs a unique container name for every container
in the DAX integration test run.
Stub in SystemAPI to Queryer (note: will not work if used)
This just makes is so that dax can compile. Actually implementing
system-table functionality for dax will take some planning.
Tlt/dax merge prep (#2282)
* Remove copilot directory
* Remove Dockerfile-datagen-long
* Remove orphaned RegisterNodeRequest
This type is not defined in the dax/mds/http package.
* implement TIMEQUANTUM and TTL in Table.Field type
* Remove the "service" misdirection in queryer/writelogger/snapshotter.
We had originally used an additional layer, er.. package, for a "service".
The main distinction was that the Config differed in that it was
internal, unlike the Config that we need to provide for the top-level
server config (i.e. toml). Having that additional layer just to support
a different Config seemed premature at best. So I'm removing it.
* Remove dax docker containers no longer used in tests
Since we run everything as "featurebase", we don't have multiple
container types anymore.
* Some minor comment updates
* Remove nfpm stuff related to dax
* Fix linter issues
Fix "duplicate" issues raised by sonarcloud.
run docker components of dax integration tests with coverage
trying to get dax integration coverage
add coverate volume mounts throughout dax integration tests
add a lock, tweak dax Makefile, remote flag on query handler
remove some unused code
convert batch tests to use clustertests to get coverage
maybe fix clustertests
more authclustertests fixes, test is failing locally
but also seems to have been silently failing in CI prior to these
changes... let's see if it's still silent
fix some lint to kick CI
just re-running the job wasn't working... strange behavior
remove RetryLogic test and pipe which don't work
RetryLogic test removed due to etcd changes. Seebs thinks we shouldn't
test this here.
Pipe was being ignored since we're no longer using "bash -c" to
execute the command. If we need to generate that output file we'll
either have to reintroduce bash -c and set -o pipefail so that it
actually fails properly, or figure out some other solution.
shooting into the dark...
first cut at bulk node registration
remove unused stuff from batch tests, set coverpkg to ../...
batch registration timeout and fix tests
disable most tests and don't run fb background batch test
debuggin!!!!!!!!!
and then he tried this....
Implement importer (for INSERT INTO) in the Queryer
Prior to this, we we passing a nil value in for the importer to the
planner.NewExecutionPlanner in the Queryer. This meant that INSERT INTO
statements didn't work. Now they should.
It uses the importer that we build for IDK in /idk/mds/importer.go, and
wrapps that with a type that can determine if the provided string
"index" is of the form indexName or TableKey.
turn off debug mode, fix log saving
Run sql3 test definitions in a dax integration test
There are currently 22 tests which are not passing. They are skipped in
the "skips" slice.
WIP, not working, pql queries to tests
Add TableQualifier to PQL query logic in the Queryer
Add more PQL tests to the keyed table
Allow instant node registration if registration-batch-timeout=0
When running dax services in process, we don't want to wait 3s for the
compute node to register; we know it's there because it's in the same
process.
Fixes related to IncludesColumn PQL test.
Tests for ConstRow and FieldValue
cleanup
add UnionRows and Options, better error reporting on bad queries
delete unused schemar client.go, clean up unused in batch test CI
move test timeouts into more reasonable territory
apparently this had already been done, but got merge-stommped at some point
move dax bolt test helpers into dax package
Add computer CheckIn routine (#2296)
* Add computer CheckIn routine
This adds a background routine which sends a "check-in" request to MDS
every <interval>. This is to address the case where the poller has
removed a computer node from the node list (due to a network fault, for
example), but the node is still healthy and becomes available again. In
that case, the node needs to "check-in" to tell MDS it is still there.
MDS will likely send the node a new directive with Method=reset telling
the node to delete all of its data an apply the latest directive.
* Don't send directives to Deregistered (i.e. removed) nodes
We have an issue where we're locking on sendDirective in the
controller, and when the node is unavailable, the send hangs and never
releases the lock. This is a temporary fix for that until we address the
real problem.
Fix .gitlab-ci.yml after rebase
fix some indentation shenanigans
(cherry picked from commit 20a8b5713a)
Concurrency was previously fully disabled due to duplication when using '--auto-generate' but testing has shown that it works correctly when not using that flag. Added the required check and updated help text
(cherry picked from commit ba70bee875)
At 50ms, we see sporadic failures in CI. So much for "this should
only need a couple milliseconds". Bumped timeout to avoid that.
The challenge here is that we have some tests which *want* to hit
the timeout to confirm that we aren't allowing things we shouldn't.
But we don't want the test to hang forever. But we want to be sure
it is actually stuck and not just being slow...
(cherry picked from commit f1a68f8a82)
SQL BULK INSERT
This change is to support a BULK INSERT/REPLACE statement that adds the ability to 1) take its input from a file, url or in-line blob 2) to map from the input source to the target columns
3) to transform data (using sql expressions) before inserting
4) support csv and ndjson formats
* improving test coverage
* increase test coverage again
* refactoring for handling transformation with types other than id and int
(cherry picked from commit 8f660a5033)
* Update package scripts so they restart services based on the operation (update vs install)
Co-authored-by: Julio Martinez <julio.martinez@logicmonitor.com>
(cherry picked from commit e25e395bab)
For unrelated reasons, we renamed the protobuf interface from
package "pilosa" to package "proto". This means our GRPC endpoints
need to live under "proto.Pilosa" instead of "pilosa.Pilosa". This
means lattice needs to be configured the same way.
(cherry picked from commit 3a8d08fed5)
* implement system tables that contain internal state information from FeatureBase
* review feedback
* review feedback
* removed sys prefixes
(cherry picked from commit 45067b4e32)
userInfo at the relevant line can be nil here. We check later if the
userInfo != nil (and assure it passes authn/authz) or if userInfo == nil
then we return all the indexes we can find.
(cherry picked from commit 407d52baa0)
This fixes the OriginalIP and RequestUserID in the main featurebase
package, and the Access and Refresh tokens, the UserInfo, and the
[]string of Indexes passed with context.Context(s) in the authn package.
An empty struct was used for all of these keys (and relevant helper
functions we added) to avoid allocations where possible while still
using the context functionality.
Some of the logic in the server.GetIndexes function was fixed.
(cherry picked from commit 0f5a56c958)
This provides us with most of the existing Tx interface, split
across QueryRead and QueryWrite. The functions not included here
are the ones that are used *only* for anti-entropy (ForEach
and ForEachRange).
We add additional testing to verify that TxStores are getting
closed correctly, to go with cleaning up the test directories they're
made in.
We also introduce some test wrappers that can automatically
fail tests on error, so tests don't need to be full of error
checks.
Also, now that I'm starting to think more about the flow of
writing tests using QueryScope, we add the missing "full
database" scope option, and make the Add methods return
their operand so (1) you can chain them, (2) you can use
the AddIndex(...) inline in a NewWriteQueryContext.
Also addressed a plausible performance concern in shardList,
and some comments that were stale or incorrect.
The test coverage here is skimpy on the actual RBF-calling
functions because those are trivial. We do, however, significantly
expand coverage in the random write requests, which are now
a mix of random writes and random reads, and add test cases
that at least hit a lot of the error checks once.
The Error() method is changed to be like (testing.T).Error(),
taking ...interface{} and using fmt.Sprint on them.
There's also some minor tweaks such as making the visualizations
more consistent, testing visualization generation on two kinds
of keysplitter, and so on.
(cherry picked from commit 7b434cd11c)
I was wondering why this is exported, and the answer is, if it
weren't exported, staticcheck would have reported that it was unused,
which it is. We don't need a wrapper on os.MkdirAll that we never
use.
(cherry picked from commit e9a40cdc35)
The anti-entropy feature has never actually worked. We've been
talking about removing it or replacing it for ages, but haven't
had a concrete motivation.
But the anti-entropy interface is the sole user of several components
of the Tx interface, and now that we're trying to replace that
interface, being able to drop those components has some appeal, so
let's remove the one thing that used them, in the hopes that this
will simplify life.
This also lets us drop ForEach and ForEachRange, which were
barely used at all. The one surviving usage (CSV export) can be
handled by using the container iterator we already have, and
making ContainerCallback exported so we can use it to just call
things for every bit.
(cherry picked from commit fff9ddc1f5)
refactored comparison, equality and arithmetic expr eval for decimal data types and added a test to cover expression eval for inserts
fixed failing test
(cherry picked from commit 193ef7cba1)
The internal/ingest and internal/schema endpoints were developed with
intent that they'd be the primary interface new users would work with,
because they were Easy To Use, and did not require any kind of setup,
the counterpoint being that ingest done this way had performance issues
because it ended up with huge amounts of JSON parsing to reformat
things into our native format. But this was understood to be the price
of providing a new-user-friendly JSON ingest experience.
A year later, we have no evidence that it's ever been used. We never
even moved it out of the `/internal` path. It's a lot of very complex
fiddly code and we don't seem to be using it, and at this point, our
anticipation is that if we really need something, we'll use CSV, which
we already have working, or something in the new SQL code. Either way,
we don't seem to be using this.
(cherry picked from commit 16ccbc461a)
We think etcd's tendency to mistakenly mark nodes down may have
been addressed. We can't find out without checking for it.
The exact pool of methods in methodsDegraded may have bitrotted
some; for instance, it didn't have PastQueries or PartitionNodes
in it, but it looks like it reasonably should.
We rework the Replica1/Replica2 server tests to reflect the
intended semantics again.
(cherry picked from commit 164dec1703)
The special case of Starting allowed us to make sure every node in a
cluster waited for the whole cluster to come up, but caused problems
later if a node died and came back. We drop the Starting state for
clusters, treating a STARTING node as equivalent to an UNKNOWN (or
DOWN) node for purposes of cluster state, so clusters will go from
Down to Degraded to Normal as nodes come up. We now wait for the
Normal state during initial bringup. We would previously have accepted
Degraded, if you could reach it, for instance if a node came up and
then went down again before another node finished starting, but I'm
pretty sure that was unintentional.
This solves a problem where while a node was down, we'd accept
queries that we could handle in a degraded state, but then we'd
*stop* accepting them when the node started coming back up.
(cherry picked from commit 52d3329434)
* Formatting adjustments made during code review.
While reviewing the BULK INSERT logic (in order to decide how best to
approach "ingest via sql" in the cloud), I made a few formatting and
comment changes. I'm just adding them here as a separate commit so they
don't muddy up my actual work.
* Parser modifications to support mulitple tuples in INSERT INTO
This commit doesn't include all of the changes required in the
planner. Fow now, the planner is simply modified to continue supporting
a single tuple (the first tuple in the list).
* Update the planner to handle multiple INSERT INTO tuples
This is part 1. It's still using the existing logic which builds an
ImportRequest for every record (and every field!).
The next step will involve using a client.Batch to handle the records.
* Introduce client.Importer interface (used by client.Batch)
Instead of the Batch having a pointer to a client, this puts an
interface there instead (which the client implements). It also allows us
to inject a different importer (i.e. other than a featurebase.client)
into the Batch.
* Decouple batch from client
This commit pulls batch-specific code out of the client package and into
a new batch package. It introduces the batch.Importer interface, the
methods of which replace all the calls that batch was previously making
directly to client methods.
Finally, it contains two implementations of the batch.Importer
interface: one is a wrapper around client, and the other is a wrapper
around featurebase.API.
* Use docker (instead of MustRunCluster) for internal batch tests
Because the `batch` package tests are internal, using
test.MustRunCluster() resulted in an import loop (because it eventually
imports `server`, and we can't have that). So this commit replaces the
use of `test.MustRunCluster()` with docker. The setup is basically the
same as that used in the idk docker tests.
Here we also remove all client-side references to `UseIngestAPI`, which
is an experimental (json) ingest api. It's still suppored on the server,
but here we remove the external usage of it.
* cherry-pick fix
* Use batch.Import() for sql3 INSERT INTO statements
* Thread logger into sql3
* fix batch test
* Fix some shadowing complaint by linter
* Address some test issues related to stringsets
* Exclude batch integration tests from CI
* Address PR feedback
- Added description to batch.README
- Consolidated grep commands in .gitlab-ci.yml
- Removed some debugging comments
- Replaces some inadvertantly removed license headers
* Add batch package to gitlab CI
* Updated CI for batch package
Updated CI include path
Update gitlab ci
Update CI
Update CI
Trying new include path for ci
Updated gitlab ci include path
Made idk race job optional for sonarcloud upload
add testdata directory
remove testenv from dockercompose file
use GIT_STRATEGY clone in batch CI
add testdata volume to dockercompose
Co-authored-by: Fletcher Haynes <fletcher.haynes@generalassemb.ly>
(cherry picked from commit 00ef2380e5)
This is living in a subdirectory for now so we can have better
turnaround time on tests and not have to build everything else
along with it.
This covers the logic that we can have *without* actually using
databases or the filesystem in any way, just to provide a framework
that lets us validate the logic handling overlapping queries.
The overall purpose of this is to prevent deadlocks, by ensuring
that database locks are only taken when we have already proven
that they are available. In short, the QueryContext preregisters
its "scope" -- the set of things it may want to lock. The operation
of creating the QueryContext can block, but it blocks with no
database locks held. Once it is unblocked, the scope it has reported
is now considered unavailable, and no other QueryContext using any
overlapping scope can complete creation until this QueryContext
completes. While it's running, the QueryContext can't request write
access to anything outside its scope. Thus, once created, a
QueryContext can always proceed, without being blocked, until it's
done.
Note that this does not fully address multi-node behaviors;
once you have a QueryContext blocking things, you need to not
make queries to other nodes that could be blocked in turn by those
nodes. In short, no write queries to other nodes while holding a
write-type QueryContext on the local node, because if two nodes
do that to each other at once, they can both be blocked.
We believe RBF is currently designed such that read-only accesses
don't block progress on writes, so non-write access doesn't
create problems.
We also have some code to allow us to create dot-format output
from the components of this system, which is mostly intended to
be a debugging tool.
(cherry picked from commit e3d137f29c)
Covers tightening up handling filter expressions that contain is/is not null ops. These filters may have to be translated into PQL calls to be passed to the executor and even though sql3 language supports nullability for any data type, currently only BSI fields are nullable at the storage engine level (there is a ticket to add support for non-BSI field here FB-1689: IS SQL Argument returns incorrect error) so when these fields are used in filter conditions we need to handle BSI and non-BSI fields differently.
enriched metadata for tables
added support for the concept of a table and field owners in metadata; mechanism to derive owner from http request metadata; metadata for table description
We thought stack traces were mildly expensive. We were very wrong.
Due to a complicated issue in the Go runtime, simultaneous requests
for stack traces end up contending on a lock even when they're not
actually contending on any resources. I've filed a ticket in the
Go issue tracker for this:
https://github.com/golang/go/issues/56400
In the mean time: Under some workloads, we were seeing 85% of all
CPU time go into the stack backtraces, of which 81% went into the
contention on those locks. But even if you take away the contention,
that leaves us with 4/19 of all CPU time in our code going into
building those stack backtraces. That's a lot of overhead for a
feature we virtually never use.
We might consider adding a backtrace functionality here, possibly
using `runtime.Callers` which is much lower overhead, and allows us
to generate a backtrace on demand (no argument values available,
but then, we never read those because they're unformatted hex
values), but I don't think it's actually very informative to know
what the stack traces were of the Tx; they don't necessarily reflect
the current state of any ongoing use of the Tx, so we can't necessarily
correlate them to goroutine stack dumps, and so on.
* resolving bool null field ingestion error
* testing issues
* adding null support for bools
* updating the null bool field ingestion
* trying to resolve issue when ingesting null value for bool type
* adding a clearing support for bool type
* resolving issues with bool null value ingestion
* updating the jwt go package version and removing changes made in docker compose file
* reverting jwt go version
* removing v4 of jwt
* adding a comment in test file to see if sonar cloud accepts this file
* adds dockerfile for building images to run examples on
* adds makefile entries to build and push docker runner images
* cleans up tar artifacts when building ingest runner
* moves executables to /usr/local/bin
* moves the sources for building runner images into their own folder
* simplifies makefile and adds directions to readme
* use t.Fatal(f) to abort tests, not panic
* make perf_able run at all, make it debug a bit better
switch perf-able to using same node type we use for other spot instances,
because otherwise it never finds any available capacity.
we switch the perf-able script to use the standard get_value function
instead of direct jq calls.
we try to grab server logs if the restore fails in the hopes of finding
out why the restore very occasionally fails.
* Fix some issues with running IDK tests in docker. (#2248)
*Stop running TestKafkaSourceIntegration with t.Parallel()
This test can't be run in parallel as it's currently written. Doing so
allows for interleaving of messages to the same kafka topic between
tests.
I didn't attempt to modify the test so it could be run in parallel. That
could be done, but left for someone more ambitious.
* Remove idk/testenv/certs which got accidentally committed.
also update .gitignore to include those.
* changes to add bool support in idk (#2240)
* initial changes to add bool support in idk
* modifying some default parameters for testing, will revert them later
* adding support for bool in making fragments function
* boolean values implementation without supporting empty or null values at this point
* Implement bool support in batch using a map (and a slice for nulls) (#2247)
* Implement bool support in batch using a map (and a slice for nulls)
* Keep the PackBools default for now
But set it explicity in the ingest tests which rely on it.
* Modify batch to construct bool update like mutex
The code in API.ImportRoaringShard has a switch statement which causes
bool fields to be handled like mutex fields. This means, that the
viewUpdate.Clear value should only contain data in the first "row" of
the fragment, which it will treat as records to clear for *all* rows.
This makes more sense for mutex fields; for bool fields, there's only
one other row to clear. But since the code is currently handling them
the same, we need to construct viewUpdate.Clear such that it conforms to
that pattern.
This commit also adds a test which covers this logic.
* Remove commented code; revert config for testing
This commit also removes the DELETE_SENTINEL case for non-packed bools,
since that isn't supported anyway.
* Revert default setting
* remove inconsistent type scope
* correcting the logic of string converstion to bool
* resolving an error in a test
* adding tests to cover code related to bool support in batch.go file and interface.go files
* modifying interfaces test
* added one more test case
Co-authored-by: Travis Turner <travis@pilosa.com>
Co-authored-by: Travis Turner <travis@molecula.com>
* resolving bool null field ingestion error (#2254)
* resolving bool null field ingestion error
* testing issues
* adding null support for bools
* updating the null bool field ingestion
* trying to resolve issue when ingesting null value for bool type
* adding a clearing support for bool type
* resolving issues with bool null value ingestion
* updating the jwt go package version and removing changes made in docker compose file
* reverting jwt go version
* removing v4 of jwt
* adding a comment in test file to see if sonar cloud accepts this file
* don't obtain stack traces on rbf.Tx creation
We thought stack traces were mildly expensive. We were very wrong.
Due to a complicated issue in the Go runtime, simultaneous requests
for stack traces end up contending on a lock even when they're not
actually contending on any resources. I've filed a ticket in the
Go issue tracker for this:
https://github.com/golang/go/issues/56400
In the mean time: Under some workloads, we were seeing 85% of all
CPU time go into the stack backtraces, of which 81% went into the
contention on those locks. But even if you take away the contention,
that leaves us with 4/19 of all CPU time in our code going into
building those stack backtraces. That's a lot of overhead for a
feature we virtually never use.
We might consider adding a backtrace functionality here, possibly
using `runtime.Callers` which is much lower overhead, and allows us
to generate a backtrace on demand (no argument values available,
but then, we never read those because they're unformatted hex
values), but I don't think it's actually very informative to know
what the stack traces were of the Tx; they don't necessarily reflect
the current state of any ongoing use of the Tx, so we can't necessarily
correlate them to goroutine stack dumps, and so on.
* fb-1729 Enriched Table Metadata (#2255)
enriched metadata for tables
added support for the concept of a table and field owners in metadata; mechanism to derive owner from http request metadata; metadata for table description
* tightened up is/is not null filter expressions (FB-1741) (#2260)
Covers tightening up handling filter expressions that contain is/is not null ops. These filters may have to be translated into PQL calls to be passed to the executor and even though sql3 language supports nullability for any data type, currently only BSI fields are nullable at the storage engine level (there is a ticket to add support for non-BSI field here FB-1689: IS SQL Argument returns incorrect error) so when these fields are used in filter conditions we need to handle BSI and non-BSI fields differently.
* added a test to cover the keyword replace as being synonymous with insert (#2261)
* update molecula references to featurebase (#2262)
Co-authored-by: Seebs <seebs@molecula.com>
Co-authored-by: Travis Turner <travis@pilosa.com>
Co-authored-by: Pranitha-malae <56414132+Pranitha-malae@users.noreply.github.com>
Co-authored-by: Travis Turner <travis@molecula.com>
Co-authored-by: pokeeffe-molecula <85502298+pokeeffe-molecula@users.noreply.github.com>
Co-authored-by: Stephanie Yang <stephanie@pilosa.com>
* removes build time constant in favor of single constant with relevant godoc and warnings
Co-authored-by: Christopher Lowenthal <christopher.lowenthal@molecula.com>
* removes rewrite-rules, sets gofmt.simplify to default true
* moves go vet step before the golangci-lint step
* fixes go vet issues with test files
* updates gocognit.min-complexity to default of 30
* removes deprecated options, primarily around run.go version
When we've started a fake cluster, we should expect to reach a
"STARTING" state, not a "DOWN" state. This test would coincidentally
pass as long as we checked the state before any of the nodes got
their notification from the node watcher that at least one node was
STARTING, because prior to that the cluster would be DOWN. But once
it got to STARTING, we would wait forever; we never reached the
instruction to tell the nodes to come to any other state, and they
would never reach a DOWN state.
* ID sql3 internal type representation is int64; fixed a bug that assumed incorrectly that it wasn't
* refactored some names for clarity
* primary: get nested loop joins to work; secondary get brute force aggregations for SUM working
* added tests; removed debug output
* review feedback
* Update sql3/planner/compileselect.go
review feedback
Co-authored-by: Travis Turner <travis@pilosa.com>
Co-authored-by: Travis Turner <travis@pilosa.com>
The testhook post-test hooks only work if you use a TestMain to
invoke them, otherwise the cleanups can be registered but never
actually get run. This deletes the etcd sockets, and temp
directories, that we created from our test runs. We also fix
the test creating a temp file directly to create it in a TempDir
(which gets cleaned up after the test), and fix the name of the
top-level tests displayed in TestMain.
We centralize the creation paths for test indexes, fields,
etcetera so they all have a common path, all using standard
test holders. There's still two versions, one for test.* functions
and one for internal. They do share a TestHolderConfig though.
Large hunks of the related APIs are simplified/streamlined.
* Fragments are always created with a Field and don't need
a workaround in case they don't have it.
* Creation of test fragments, etc., use optional FieldOptions
but don't specify names because they're all using new holders
for each thing created anyway. This dramatically reduces
the complexity of the calls.
* test fragments are created inside test views which are created
inside test fields, etcetera, so everything is using the same
logic; test views aren't bypassing the other layers, they're
creating themselves normally within a field.
* Quite a few things now use the standard runtime/production
logic instead of being custom workarounds; for instance, instead
of `mustOpenMutexFragment` creating a fragment and then creating
a mutex vector for it, we just create a mutex-typed field and
have the normal runtime code do this.
* Similarly, we now use the same field creation logic that production
does, instead of having our own test-only thing that validates
field names directly, so our test that we're validating field names
is actually testing the runtime code. Yay.
* fragSpec goes away. it was a replacement for fragProxy which existed
to solve memory allocation problems but replaced them with interface
overhead problems. Now we just have pointers to things and maintain
valid data structures.
* Many panics are now Fatal or Fatalf calls.
* Some specific bugs fixed, like a cluster which was requested and
then had its first node directly overwritten, which isn't valid with
shared clusters.
* Drop the temp-dir test flag and TempDir variable, we can just use
$TMPDIR.
* Drop a benchmark of "write file to disk" that was purely a benchmark
of file write speed, not a benchmark of rendering the data that needs
to be written.
* Drop the unused "flags" parameter to fragment creation, which was
only used back when we changed the BSI format.
* Use holder.Txf() rather than index.Txf(). The TxFactory has to be
holder-level anyway, referring to it via the index is misleading.
* Test holders automatically close themselves and delete themselves,
we remove various other things that thought they were responsible
for deleting themselves.
The default client appears to be pretty spammy and flood us with
debug messages about POST and GET requests, and honestly we don't really
need these or benefit from them, I don't think, so let's not.
A few view functions were taking a Tx, which had to be shard-specific,
but that's sort of awkward -- the view is inherently not shard-specific,
so it should be handling sharding internally.
There were also a couple of remaining obsolete checks for whether a
Tx was nil, at least two of which were in contexts where it absolutely
can't be. Remove all of them, and also the function itself.
* handle multi field count correctly
COUNT() should ignore null values.
If the data type of the expression supports an existence bitmap for the underlying FeatureBase data type we will use it to eliminate nulls from the aggregate
* simplify aggregate for existence test
we can use a direct != null instead of an indirect not(=null), and
avoid relying on the probably-broken behavior in the executor that
tries to silently fix up Row(x=3) tests on BSI fields which wanted
Row(x==3).
Co-authored-by: Seebs <seebs@molecula.com>
We had this fail in CI once, and failing took 30 minutes because
we didn't have a timeout on this. This shouldn't ever fail, but
the fact that it did indicates that the fabled etcd failures
we've seen a couple of times were still capable of happening.
This will make that failure happen sooner and more clearly.
Also, log the cluster states (and possibly node states) while
waiting. But add a delay -- otherwise we can do this quite a few
times per millisecond. We use Logf so that, if you didn't use -v,
you see these reported only if the test fails, but if the test fails,
we'll say what happened.
It would probably be better to have a passive thing that can wait
for updates, because we're waiting on heartbeats. Missing: A way to
detect what's actually happening in the failure cases, which we
see only quite rarely.
This is a bit complicated and entangled, sorry.
First, we squash the auth-based smoke tests into the regular smoke
tests; we just run all the tests with auth on and that way we don't
need to spin up an entire separate cluster of machines just to run
a single query against them.
We improve the error detection, and standardize the jq-to-get-config
code. The purpose of this is to try to make sure that, if we actually
hit a failure and get "null" for a host name, we report *that*
as an error, rather than running ahead and producing 20+ separate
reports that ssh failed because it couldn't find a host named null.
You have to start the cluster before you can refer to its holders,
because GetNode doesn't work on an unstarted cluster, but if you
actually issue any commands, those require messing with the worker
pool which wants to have access to the holder's stats.
"featurebase cli" will now save command history to
$HOME/.featurebase/cli_history by default. Additionally if a command
is entered across multiple lines, the newlines will be removed when
the command is saved in the history. Previously each line was saved
separately which was a bit annoying.
We were using v1.2.0 of the github.com/satori/go.uuid library to
generate UUIDs for transactions if the transaction had no previous id.
That version of the library had CVE-2021-3538: "Due to insecure
randomness in the g.rand.Read function the generated UUIDs are
predictable for an attacker."
More reading can be done here:
https://pkg.go.dev/vuln/GO-2022-0244https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-3538
This vulnerability was found using the new govulncheck tool which is not
currently used in our CI pipeline but might be a good candidate to
include in the future. (Like all tools like this there are caveats to
its usage and utility which can be read about below.)
Information on that tool can be found here:
https://go.dev/blog/vulnhttps://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck
* first cut of working (slowly) bulk insert; table valued functions and a tuple data type to support time quantums
* oversight
* filter pushdown implementation; bulk insert
* addressed some linter issues
When doing tests, we create a ton of one-off clusters. This
turns out to be expensive and slow. Fixing it is surprisingly hard.
Fundamentally: If we're sharing clusters, we need to use different
indexes for each test, to avoid clashes. This changes index names.
As a side-effect, this reorders many partition-based things, like
the order keys are returned in. Thus, to fix this, we change a lot
of tests to no longer depend on the *order* in which strings are
returned.
Having done that, we can also discard the ModHasher behavior, since
that only existed to allow us to reliably predict partitioning.
The basic design is as follows: Instead of a cluster being a
[]*Command, a "shareable" cluster is now a []*Command plus some
flags, and a "cluster" is a pointer to a possibly-shared cluster,
plus a link to the specific test using this specific cluster,
and correspondingly, its test name suitably coerced to be a valid
index name prefix.
The "test.Cluster" object now has methods to allow retrieving an
index name, and also implemnts fmt.Formatter to let you use,
e.g., `%i` with it in Sprintf to get "the index name, plus an i".
(This works for everything but %p and %T.)
This allows us to consistently rework all the many things that
use index names in a persistent way.
We also have `MustUnshared` and `MustRunUnsharedCluster` methods
which allow us to specify that a given test needs its own cluster
for some reason. For instance, the tests that want to run backups
need their own isolated cluster, and the tests that want to close
or reopen nodes need their own cluster because a reopened cluster
won't have working GRPC for some reason.
On "closing" a shared cluster (actually the test-specific wrapper
that reflects a given sharing), we delete any indexes starting with
that test's index name prefix. Otherwise, the huge pile of open
indexes prevents `go test -race` from working on MacOS, where we
run out of address space too quickly.
This is fairly enormous but most of the individual changes are
fairly trivial things like replacing the string "i" with "c.Idx()".
We also tweaked a test that failed for me a couple of times to
not depend on sort order.
We reuse a fragment. It might seem surprising that this works, but
the fragment code actually doesn't have a persistent bit depth at
all, it just accepts whatever bit depth you tell it to use. Cutting
out the recreation of the fragments saves some time.
We also cap bit depth at 8, instead of 62, because there's a ton
of runtime cost to testing more bit depths, but it doesn't actually
change the logic.
For arbitrary mod values m, greater than zero,
(x%m + 1) != 0
is always true
What we almost certainly meant was
x%(m+1) == 0
which would give you all the bits in row 0, half the bits in row 1,
etcetera.
Also, we drop to doing a quarter-shard because why not.
This test used to be large, because it was testing some features that
were refactored out in October of 2019. Since we no longer have the
"buffer growth" to check, let's check a much smaller file.
We want to be able to register hooks which do cleanup, which may be
registered after the auditor cleanup check, which means that we
want LIFO order for post-hook cleanups.
We also want the test hook cleanup to be deferred, rather than
merely run after the tests are executed. Also, we have to extract
the result from running the test, then execute deferred things,
*then* call os.Exit, because os.Exit bypasses defers.
The "field/view will just synthesize a tx" behavior is awful and
also hides a number of fundamental flaws. We distinguish between
"we really do mean to work on a single shard here" and "we intend
to work on the whole field or view", and the latter now take
Qcx instead of Tx.
This eliminates a lot of very weird cases where we checked for
nil Tx and synthesized them, and also gets us away from
field and view taking Tx parameters when no possible Tx
can be constructed which is valid, because Tx are inherently
shard-specific at this time.
* squashed 45 commits into one :)
* tlt/sql experiment (#2035)
* Move PlanOperator to sql3/planner/types package
includes:
type PlanOperatorColumn struct
type PlanOperator interface
* Remove planner dependencies from pilosa package
The goal after this is to prevent the planner package (which doesn't exist yet)
from being imported by the pilosa package; we just want it injected into the server
in server/server.go. This is because the planner package uses pilosa types, so we need
to avoid circular dependencies.
Added ExecutionPlannerFn
Make public: pilosa.ExecOptions
Added a pilosa.Executor interface
Added a planner.types.CompilePlanner interface
Isolated the planner calls to:
- Executor.Execute()
- *API.[method]()
* Move executionplanner files into the sql3/planner package.
This required a bit of gymnastics, and there are some things around
FieldOptions which need to be addressed soon.
* Remove the hacky FieldOptions stuff I added earlier
This implementation just uses the pilosa.FieldOption functional options
provided by the API (as opposed to trying to build a FieldOptions
object.
It also changes field types to constants. These are private for now, but
if we need to make them public, we should put them in the planner/types
package.
* Implement the "scale" value from Decimal(scale)
Also, precision and scale were currently reversed in the parser. This
fixes that.
* Modify the parser to handle CACHETYPE <type> SIZE <size>
It's a little odd to me that the cache type values are Tokens, but I
guess it's ok. One thing to keep in mind is that FeatureBase expects
lowercase values, so this commit changes the parser to set the value to
the lowercase version of the type.
* Fix the /sql2 tests
This entailed a combination of commenting out or t.Skip()-ing tests
which covered code in the parser that has been commented out or removed
as not currently supported in sql3.
It also adds some coverage for the sql.Contraint stringers.
* Prevent JSON sql results from containing closing commas
This commit just re-works the existing output code to avoid inserting
closing commas (which results in invalid JSON).
* Enhance the CREATE TABLE test coverage.
In particular, ensure that the fields which get created in FeatureBase
are what we expect based on the fields defined in the CREATE TABLE
statement.
This also ensures that the TIMEQUANTUM and CACHETYPE contraints are not
provided for the same field (since those constraints are not supported
together).
* Adjust the EBNF file to indicate SIZE contraint is optional
A CACHETYPE can be provided without a SIZE. This change indicates that
SIZE is optional.
* Remove `executionplanner_` from file names (#2040)
* implementation of ALTER TABLE (sans column RENAME)
* refactored expression analysis; added more robust type checking; all unary and bin ops function on ints
* added type support for expressions; full bin/unary op support; added cast; more literal support
* cast int to all other types
* all literals (except idset, stringset & timestamp) make it thru; cast to all types with int as source now works
* implemented LIKE/NOT LIKE
* Implemented IS [NOT] NULL
* Move sql2 files into sql3/parser package (#2045)
* Move sql2 files into sql3/parser package
This also removes the sql2 package.
* Fix tests which were typing _id fields as INT intead of ID
* implemented BETWEEN, NOT BETWEEN
* Add featurebase/error package (#2046)
* Add featurebase/error package
I copied the `dax/errors` package which I am starting to use in the DAX
prototype into `featurebase/errors` in order to start using it with the
sql3 package. It's basically a wrapper around `github.com/pkg/errors`,
but it uses a customer coded error.
The sql package can define its own errors based on the
`featurebase/errors` types. Then do things like `Wrap()` and `Is()`.
* Address the linter complaints: shadowed variables, unreachable code
* implemented IN & NOT IN with expression lists
* first cut of CASE
* Fixed some errors from rebase
* updated bnf; removed unused code; tightened up error handling
* first crack at basic CLI for SQL3
Use: `featurebase cli`
Still lots to do here, but for example:
> select count(*) from tremor
+--------------+
| COUNT |
+--------------+
| 1.158321e+06 |
+--------------+
* Iterate on the CLI (#2057)
Handle the errors.
Add an "exit" command.
Add some general formatting and white space.
Add termination character: ";" (semicolon)
This commit allows a user to provide multiple or partial SQL statements.
Example of multiple statements:
```
show tables; select * from foo;
```
Example of partial (multi-line) statements:
```
select *
from foo;
```
Don't uppercase the header values
* error refactoring; first cut of TOP; remove unused code; use log.Printf instead of fmt.Printf
* fixed a bug with QualifiedRef from refactoring; added bones of INSERT; removal of unused code; tightened up errors more; fixed failing tests
* single value list for INSERT
* Update bnf per discussion with Travis; INSERT now doing the requisite stuff
* Pat's eyes went square - nothing wrong with TOP, Pat needed to learn arrays again.
* improved some errors; fixed tests to suit
* send warnings back in the api; update CLI to display warnings
* start warning on stuff not implemented so we don't get bugged about it
* Tlt/sql experiment (#2063)
* Expresssion -> Expression
* Add SQL planner test
- adds a test to which it is easier to add tables and SQL statments
- un-exports all of the expression types
- removes the planner pointer from the expression types (it can be added
back later if need be)
* Fix where clause on a string field
Prior to this commit, the binary expression for a where clause on a
string field was building the call by providing a range operator which
is typically used for BSI fields. This changes it to use the call.Args
for string values.
* Update planner tests to handle multiple sql for the same results
* Reorganize SQL tests
Introduce a test/helpers package and move shared MustQueryRows into that
package.
* Add a compatibility map for field types. (#2064)
This is primarily to address the fact that ID fields were previously
incompatible with INT literals.
We should probably consider introducing a custom type for FieldType
which can be used to define compatibilities.
* significantly refactored type checking
* Handle nil (NULL) values in the sql CLI. (#2067)
go-pretty panics if the interface{} field value is nil. This replaces
nil values with a "NULL" string.
* Squash some commits
fixed a still failing test
added line, col to all error messages
refactored source handling to enable table aliases
fixed some copypasta per review
warnings for order by & topn; implemented select as a source
starting to handle in (select...); added stub for optimizer
JSON-encode the sql error and warning strings (#2069)
Error strings with unencoded characters (like double quotes) were
resulting in invalid json.
got insert working; added symbol table; added concrete optimizer; added nascent NestedLoopsOperator; rewrite "where foo in (select..." as inner join
* all about the sets (#2085)
* implemented setcontains()
* implemented set literal; insert set column values; setcontains/all/any both in expr eval and pql filters
* Convert test to use latest framework. (#2086)
* fixed some comments
* removed refactored tests
Co-authored-by: Travis Turner <travis@pilosa.com>
* Add support for Decimal fields to the sql test. (#2090)
* dates (#2094)
* return dates as strings in output; tightened up decimal type checking
* return dates as strings in output; tightened up decimal type checking
* fixed failing tests after decimal changes
* can now insert decimal values
* implemented insert for timestamp data type; implemented current_date, current_timestamp constants
* fixed some failing tests
* handle date literals from strings in insert statements
* changes from feedback
* Fix pointer method error
* sql3 API interface (#2110)
* Introduce API-related interfaces: SchemaAPI, ComputeAPI
The sql3 code was relying on the pointer: *pilosa.API in order to call
API methods directly on the local node. If we want to import and use the
sql3 package in another service (the DAX queryer, for example), we need
to be able to use an implementation of an interface for those API method
calls.
This commit introduces two interfaces, both automatically implemented by
pilosa.API:
- SchemaAPI
- ComputeAPI
* Convert sql3 code to use IndexInfo instead of Index
The sql3 code was relying on a *pilosa.Index and its methods to get
general information like index and field name, type, etc. This commit
converts everything to use a *pilosa.IndexInfo instead.
This allows us to modify the SchemaAPI interface to also return
IndexInfo instead of Index, which will be a lot easier to implement in a
non-pilosa package (like DAX); creating a *pilosa.Index requires
providing things like data directory paths and holders, which are not
necessary for these use cases.
* Unary and Binary Ops R US plus CAST (#2111)
* implemented string literal for timestamp epoch
* fixed failing test
* fixed the failing test again
* refactored tests; implemented unary op tests for all datatypes; implemented binop tests for int/int, int/id, int/decimal & ID/int
* implemented all binary ops for INT & all other types, ID & all other types
* implemented binary ops for DECIMAL types & all other types
* added STRING & BOOL to various tests; implemented all remaining binOp tests
* fix up some stuff after rebasing
* refactored test defs into multiple files; implemented CAST for every datatype
* added tests for like/not like
* addressed review feedback
* addressed type review feedback
* tightened up IS [NOT] NULL behavior plus tests (#2118)
* tightened up IS [NOT] NULL behavior plus tests
* BETWEEN/NOT BETWEEN with all data types
* addressed review feedback
* Handle negative integers in column min/max constraints (#2120)
This commit parses the min/max contraint as an expression, as opposed to
an int literal, so that negative values are treated as Unary
expressions.
There currently isn't support for min/max constraints on `decimal`
fiels, so for now this change only expects +/- integer values.
* Implement the CREATE TABLE keypartitions logic (#2123)
* Execution time, IN/NOT IN & multiple aggregates (#2124)
* added display of execution time
* IN/NOT IN tests for all data types
* fixed date parsing
* removed duplicative tests
* refactoring aggregates
* suport multiple aggregates
* Address review feedback
* final round of feedback
* Add method SchemaAPI.CreateIndexAndFields() (#2127)
In order to support a CREATE TABLE statement as a single command, this
commit alters the SchemaAPI interface to contain a single method which
handles both the index and its fields. It also updates the sql3 code to
use this interface instead of CreateIndex() and CreateField()
indepedently.
* Symbol Handling (Again) (#2129)
* Refactored symbol handling in the planner; re-instated the select as source tests
* removed commented out code
* addressing review feedback
* Move hard-coded _id field out of planner and into interface implementation (#2130)
This commit moves the hard-coded addition of the `_id` field from the
planner to the SchemaAPI.IndexInfo() implementation method.
NOTE: If anything was expecting SchemaAPI.Schema() to also return the
`_id` field as part of its field list in each table, then it would not
be there because the `_id` field is only added in the IndexInfo() method
for now. Currently that's not a problem because nothing is expecting the
`_id` field for `Schema()`.
* Multiple aggregates, all aggregates stand alone and in GROUP BY (#2132)
* handle multiple aggregates in group by queries
* added handling for avg() aggregate both stand alone and in group by
* tightened up sum & avg outside of group by
* added min, max & percentile
* added warnings
* Make MaterializedRowSet implement the PlanOperator interface. (#2133)
This commit refactors the PQLMultiGroupByOperator to have a PlanOperator
as its output. Then, when it initializes, it sets up a
MaterializedRowSet and populates that with the values from the multiple
group by operations.
* added explicit min/max pql operators
* saved a file I forgot to save
* per review
* Un-indent some if/else nesting (#2136)
Co-authored-by: Travis Turner <travis@pilosa.com>
* Add optional `name` argument to test structs.
This commit adds the `name` argument to `tableTest` and `sqlTest` so
that a test can be optionally named. This allows a developer to more
easily run/identify a particular test by name.
* Inbuilt functions (redux) (#2141)
* set functions type parameter type checking
* implemented datepart
* include SQL3 type in SHOW COLUMNS output
* fixed select as source; failing SHOW COLUMNS test
* select in select list
* dump output columns; handle optimization for select list subqueries
* make it an error to return multiple rows for a select list subquery
* added description
* contants and test coverage for datepart function
* SQL3 Refactor-palooza (#2182)
* removed unneeded IsAggregate()
* first cut of working nested loops operator aka INNER JOIN
* remove selectListItemPlanExpression
* added some warnings
* all the tests are passing again!
* addressed some linter complaints
* added basic order by
* bug fixes; added 'or replace'/'replace' to insert
* for insert references should return appropriately
* added back ability to use subquery singleton expressions
* removed dead code; fixed test
* json-able plan, Schema() plus refactoring
* fixed dumb code
* add some tests for time quantum behavior
* Code cleanup during review. Also fixed INSERT to keyed table bug.
This commit contains a lot of minor adjustments made during code review.
It also contains a bug fix that was preventing INSERT into a keyed table
(i.e. _id type STRING) from working.
Co-authored-by: Travis Turner <travis@molecula.com>
* Fix expected min/max on timestamp column test (decimal field)
I don't know why this changed, but presumably something to do with
decimal related work that happened on master.
* Fix compile problem after rebase
* review feedback
Co-authored-by: Matthew Jaffee <jaffee@pilosa.com>
Co-authored-by: Travis Turner <travis@pilosa.com>
Co-authored-by: Travis Turner <travis@molecula.com>
Co-authored-by: Fletcher Haynes <fletcher@capitalprawn.com>
* removes unused filesize function
* removes ioutil usage
* updates ioutil.ReadAll to io.ReadAll
* updates ioutil.TempFile to os.CreateTemp
* updates ioutil.TempDir to os.MkdirTemp
* updates ioutil.ReadAll to os.ReadAll
* update ioutil.WriteFile to os.WriteFile
* updates ioutil.Discard to io.Discard
* updates ioutil.ReadDir to os.ReadDir where applicable
* removes unused code in idk
* creates type to use for context value keys
* replaces assert.Nil with assert.NoError for error checks
We only have 4 bytes for offsets, but what if a file is
over 4GB? Someone came to us with a file with 265 *million* containers,
in a single fragment, which means that over 3GB of their 4.7GB file
is actually just the container headers alone. But we can't easily make
the offsets larger, or change the file format.
So we don't. We just track how many 4GB hunks of the file we've
been through and bump that every time the 32-bit offset wraps. And this
appears to... just work.
This is fixed for both the roaring iterator and the old unmarshalBinary
logic. The logic to handle this will work on 32-bit hosts in the sense
that it will correctly error out for excessively large file sizes or
container counts, but it doesn't actually handle the large files since
it can't.
When the interval is a proper superset of the range with start equal to
interval start, the range must be considered a superset or it will be
completly ignored (since it neither a subset nor it overlaps)
Without this, a data set with a ludicrously large value in it
could break a BSI field's depth even though the import would then
reject it.
always treat BSI fields as having at least their depth:
If you imported only small values, BSI fields could end up
not bothering to clear higher bits in existing values, which
produced strange behaviors.
This is the checklist that the reviewer will follow while reviewing your pull request. You do not need to do anything with this checklist, but be aware of what the reviewer will be looking for.
- [ ] Ensure that any changes to external docs have been included in this pull request.
- [ ] If the changes require that minor/major versions need to be updated, tag the PR appropriately.
- [ ] Ensure the new code is [properly commented](https://github.com/golang/go/wiki/CodeReviewComments#doc-comments) and follows [Idiomatic Go](https://dmitri.shuralyov.com/idiomatic-go).
- [ ] Check that tests have been written and that they cover the new functionality.
- [ ] Run tests and ensure they pass.
- [ ] Build and run the code, performing any applicable integration testing.
- [ ] Make sure PR is tagged with appropriate changelog label.
- 'declaration of "(err|ctx)" shadows declaration at'
- 'Error return value of .(.*\.Help|.*\.MarkFlagRequired|(os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*printf?|os\.(Un)?Setenv). is not checked'
Thank you for your interest in contributing to FeatureBase! We appreciate your support in making this open-source project even better. Here are some guidelines to help you get started with contributing to FeatureBase:
1. Familiarize Yourself with the Project:
- Visit the FeatureBase website at www.featurebase.com to understand the project's goals, capabilities, and features.
- Read the documentation available on the website, including the installation guide, configuration options, and data modeling concepts.
- Explore the codebase by cloning the repository and reviewing the source code.
2. Join the Community:
- Visit the FeatureBase community page at https://www.featurebase.com/community to learn more about the project's community and how to get involved.
- Join the Discord server at https://discord.gg/FBn2vEp7Na to chat with other contributors and users, ask questions, and share your ideas.
3. Set Up Your Development Environment:
- Ensure you have Go installed on your machine. Make sure your shell's search path includes the go/bin directory.
- Clone the FeatureBase repository or download it as a zip file from the repository's page.
- Follow the "Build FeatureBase Server from source" instructions in the README file to compile the server binary and the ingester binaries.
4. Choose a Contribution Area:
- Identify the area you'd like to contribute to, such as bug fixes, new features, performance improvements, documentation updates, or community support.
- Check the issue tracker on the repository or the FeatureBase community for open issues or feature requests that align with your interests and skills. Alternatively, propose your own idea by creating a new issue.
5. Create a New Branch:
- Before making any changes, create a new branch in the repository's Git repository. This branch will contain your contributions.
- Give your branch a descriptive name that reflects the nature of your contribution.
6. Make Your Changes:
- Follow the coding style and conventions used in the existing codebase.
- Write clear and concise commit messages for each logical change.
- If you're introducing new features or modifying existing behavior, make sure to update the documentation to reflect the changes.
7. Test Your Changes:
- Run the existing test suite to ensure that your modifications do not introduce any regressions.
- If applicable, write additional tests to cover the changes you made.
- Document any new testing procedures required for your contribution.
8. Submitting Your Contribution:
- Push your branch to the main repository or create a fork and submit a pull request to the main repository.
- Provide a detailed description of your changes, including the problem you solved and the approach you took.
- Be responsive to any feedback or suggestions provided by the project maintainers or other contributors.
- Once your contribution is approved, it will be reviewed and merged into the main codebase.
Please note that by contributing to FeatureBase, you agree that your contributions will be licensed under the Apache 2.0 license, which governs the project.
Thank you for considering contributing to FeatureBase! Your contributions are valuable and help improve the project for everyone.
FeatureBase Community is now archived and no longer maintained.
See our [internal documentation](https://internal-docs.molecula.cloud), which includes all [external documentation](https://docs.molecula.cloud), plus many internal-only pages, listed under the "Internal" heading in the main navigation bar.
* [FeatureBase Community Help](https://github.com/FeatureBaseDB/FB-community-help)
Follow along with the [Sample Project](https://internal-docs.molecula.cloud/tutorials/getting-started) to get a better understanding of FeatureBase's capabilities.
## Pilosa is now FeatureBase
As of September 7, 2022, the Pilosa project is now FeatureBase. The core of the project remains the same: FeatureBase is the first real-time distributed database built entirely on bitmaps. (More information about updated capabilities and improvements below.)
FeatureBase delivers low-latency query results, regardless of throughput or query volumes, on fresh data with extreme efficiency. It works because bitmaps are faster, simpler, and far more I/O efficient than traditional column-oriented data formats. With FeatureBase, you can ingest data from batch data sources (e.g. S3, CSV, Snowflake, BigQuery, etc.) and/or streaming data sources (e.g. Kafka/Confluent, Kinesis, Pulsar).
For more information about FeatureBase, please visit [www.featurebase.com][HomePage].
## Getting Started
* [Learn how to install FeatureBase Community](https://github.com/FeatureBaseDB/FB-community-help/blob/main/docs/community/com-getstart/com-getstart-home.md)
### Build FeatureBase Server from source
0. Install go. Ensure that your shell's search path includes the go/bin directory.
1. Clone the FeatureBase repository (or download as zip).
2. In the featurebase directory, run `make install` to compile the FeatureBase server binary. By default, it will be installed in the go/bin directory.
3. In the idk directory, run `make install` to compile the ingester binaries. By default, they will be installed in the go/bin directory.
4. Run `featurebase server --handler.allowed-origins=http://localhost:3000` to run FeatureBase server with default settings (learn more about configuring FeatureBase at the link below). The `--handler.allowed-origins` parameter allows the standalone web UI to talk to the server; this can be omitted if the web UI is not needed.
5. Run `curl localhost:10101/status` to verify the server is running and accessible.
### Data Model
Because FeatureBase is built on bitmaps, there is bit of a learning curve to grasp how your data is represented.
* [Learn about Data Modeling](https://github.com/FeatureBaseDB/FB-community-help/blob/main/docs/concepts/concepts-home.md)
### Ingest Data and Query
* [Learn how to ingest data from multiple data sources](https://github.com/FeatureBaseDB/FB-community-help/blob/main/docs/community/com-ingest/com-ingest-manage.md)
## Community
You can email us at community@featurebase.com and [learn more about contributing](https://github.com/FeatureBaseDB/featurebase/blob/master/OPENSOURCE.md).
Chat with us: [https://discord.gg/FBn2vEp7Na][Discord]
## What's Changed Since the Pilosa Days?
A lot has changed since the days of Pilosa. This list highlights some new capabilites included in FeatureBase. We have also made signficant improvements to the performance, scalability, and stability of the FeatureBase product.
* Query Languages: FeatureBase supports Pilosa Query Language (PQL), as well as SQL
* Stream and Batch Ingest: Combine real-time data streams with batch historical data and act on it within milliseconds.
* Mutable: Perform inserts, updates, and deletes at scale, in real time and on-the-fly. This is key for meeting data compliance requirements, and for reflecting the constantly-changing nature of high-volume data.
* Multi-Valued Set Fields: Store multiple comma-delimited values within a single field while *increasing* query performance of counts, TopKs, etc.
* Time Quantums: Setting a time quantum on a field creates extra views which allow ranged Row queries down to the time interval specified. For example, if the time quantum is set to YMD, ranged Row queries down to the granularity of a day are supported.
* RBF storage backend: this is a new compressed bitmap format which improves performance in a number of ways: ACID support on a per shard basis, prevents issues with the number of open files, reduces memory allocation and lock contention for reads, provides more consistent garbage collection, and allows backups to run concurrently with writes. However, because of this change, Pilosa backup files cannot be restored into FeatureBase.
## License
FeatureBase is licensed under the [Apache License, Version 2.0][License]
// NewFileBuffer returns a file buffer which will use an in-memory buffer, until `max` bytes have been written, at which point it will write the contents of memory to a file, and continue writing future data to the file.
// The file will be written to `temp` directory. The buffer fulfills the io.Reader and io.Writer interface
BatchMaxStalenesstime.Duration`mapstructure:"batch-max-staleness" help:"Maximum length of time that the oldest record in a batch can exist before flushing the batch. Note that this can potentially stack with timeouts waiting for the source."`
Timeouttime.Duration`mapstructure:"timeout" help:"Time to wait for more records from Kafka before flushing a batch. 0 to disable."`
"Id", "Name", "Short description", "Gender", "Country", "Occupation", "Birth year", "Death year", "Manner of death", "Age of death"
1, "George Washington", "1st president of the United States (1732–1799)", "Male", "United States of America; Kingdom of Great Britain", "Politician", "1732", "1799", "natural causes", "67"
3, "Abraham Lincoln", "16th president of the United States (1809-1865)", "Male", "United States of America", "Politician", "1809", "1865", "homicide", "56"
4, "Wolfgang Amadeus Mozart", "Austrian composer of the Classical period", "Male", "Archduchy of Austria; Archbishopric of Salzburg", "Artist", "1756", "1791", "0", "35"
5, "Ludwig van Beethoven", "German classical and romantic composer", "Male", "Holy Roman Empire; Austrian Empire", "Artist", "1770", "1827", "0", "57"
6, "Jean-François Champollion", "French classical scholar", "Male", "Kingdom of France; First French Empire", "Egyptologist", "1790", "1832", "natural causes", "42"
// TODO(tlt): we can't run this test until we get the system tables under control (i.e. sorted). Currently, fb_views is in a map with users, so the following can fail 50% of the time.
// Show tables for database by calling describe with no args.
EXPECT:| 1 | George Washington | 1st president of the United States (1732–1799) | Male | United States of America; Kingdom of Great Britain | Politician | 1732 | 1799 | natural causes | 67 |
EXPECT:| 2 | Douglas Adams | English writer and humorist | Male | United Kingdom | Artist | 1952 | 2001 | natural causes | 49 |
EXPECT:| 3 | Abraham Lincoln | 16th president of the United States (1809-1865) | Male | United States of America | Politician | 1809 | 1865 | homicide | 56 |
EXPECT:| 4 | Wolfgang Amadeus Mozart | Austrian composer of the Classical period | Male | Archduchy of Austria; Archbishopric of Salzburg | Artist | 1756 | 1791 | 0 | 35 |
EXPECT:| 5 | Ludwig van Beethoven | German classical and romantic composer | Male | Holy Roman Empire; Austrian Empire | Artist | 1770 | 1827 | 0 | 57 |
EXPECT:| 6 | Jean-François Champollion | French classical scholar | Male | Kingdom of France; First French Empire | Egyptologist | 1790 | 1832 | natural causes | 42 |
EXPECT:| 7 | Paul Morand | French writer | Male | France | Artist | 1888 | 1976 | 0 | 88 |
EXPECT:| 8 | Claude Monet | French impressionist painter (1840-1926) | Male | France | Artist | 1840 | 1926 | natural causes | 86 |
