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.
* 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>
* 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
* 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>
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.
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.
previously we allowed users to specify a granularity for timestamp
e.g. seconds, milli, micro, nano
however we converted everything to nano before we stored it.
This reduced the allowed range for all time units to what
was allowed by timestamp. For example, with second granularity
you can represent billions of years within the capacity of
int64 but with nano its somewhere b/w 100-200 years.
So now, for timeunits of seconds, milli, and micro the range
is year 0001 - 9999. These limits come from what Go
supports.
So this uses unit specific function to translate
timestamps to values and vice versa to increase
the time range.
In the process of increasing the range for timestamp and subsequent
testing, I found and addressed a few bugs:
- min/max queries were not using timestamp specific comparators so
added that.
- Values from Import/ingest come to FB as relative values to epoch
whereas other BSI fields come as actual values and then
becomes relative to their respective bases within FB. so some
specific handling of that was added.
- However! Set queries use timestamp strings which are, of course,
the actual value they designate. So they have to become
relative.
- When bitdepth is 0, Min/maxUnsigned functions did not run
resulting in a count of 0 when there
was an actual value that was 0.
Also, this removes (now) dead code and updates/adds tests.
* Revert "make pql.Decimal.Value a private big.Int field"
This reverts commit eab6174388.
* Revert "pql.Decimal for DecimalVal in ValCount&GroupCount"
This reverts commit a0c9eec410.
* Revert "Add AddDecimal support to pql"
This reverts commit 50787fd37a.
This ensures that we can't overflow when adding `pql.Decimal`s together. The
only place we can possibly overflow is when converting pql.Decimal to an Int64,
but that is a risk we have to take. Also, the only place we do this is in our
ToRowser. We could maybe change that to strings, so the presentation of data
doesn't indicate an overflow, but that is a later decision to make. It will
also involve fixing the generate-proto-grpc make command, because that's broken
rn.
* mutex clear on nil support with test
* Update client/batch.go
Co-authored-by: Matthew Jaffee <jaffee@pilosa.com>
Co-authored-by: Matthew Jaffee <jaffee@pilosa.com>
it's not something that we can send to the server (it calculates base
off of min and max), and we don't need it in the json string in order
to use it client side when building import requests.
if the same ID is added multiple times with different values, only the
last value should get set. Without this change, if the multiple
records weren't immediately next to each other, all the bits from all
the values would get set.
the ranked cache must be fully rebuilt as the BitmapRewriter doesn't
have an easy way to track which rows had how many bits changed which
we would need to update the cache.
We also needed to add a Clear method to topn cache to completely
remove old values before the rebuild as otherwise they'd sit there and
pollute the cache after it was rebuilt.
This also includes fixing a strange idiosyncrasy where the _exists
field was a set field, but didn't have its type explicitly set. This
was causing it to have a ranked cache even though that option was
turned off. Hoping this doesn't have any weird follow-on effects... or
if it does the tests catch them.
This adds a shard-based import endpoint which takes bitmap data for
all field types and imports data for the whole shard transactionally.
It uses the BitmapRewriter interface to try to intelligently allow for
setting and clearing bits simultaneously without multiple writes which
is especially helpful when ingesting into int-like fields, but also
allows clear-and-then-set behavior for set fields.
swallowing this error was a mistake... caller would not know that
ingest failed and might incorrectly report success
upstream (e.g. committing offsets to Kafka)
out of range. Previously internal server error was returned.
This is to allow for ingest to continue while logging bad values
instead of stopping ingest as we do when there is a server error.
* Change Ttl to TTL
Following go convention, acronyms should have a consistent case.
See
[Initialisms](https://github.com/golang/go/wiki/CodeReviewComments#initialisms)
This commit changes some public-facing methods, so any code importing
this package and using these methods will need to be updated.
* rewrite Ttl -> TTL
Co-authored-by: reesporte <reesedporter@gmail.com>
If non-primary host fails to process a request, retry on primary node.
conditions when we should not do this:
- no error
- we've aleady tried the primary
- we're making a status request to get the primary node...this
could lead to lock contention if we allow it to happen as we are
making an http request within an on going http request to discover
the primary node.
This also deletes the RemoveHost method and the associated test b/c
it is not used anywhere anymore and updates the returned error type.
- Add auth-token for featurebase import, backup and restore
- Add auth-token to http request
- Create a cluster tests with auth enabled
- Add test for import with auth enabled
the shardwidth22 tests were broken client side, but we didn't realize
this because we weren't running the client side tests since moving the
client code into the main FB repo until recently (woops), and more
recently, we'd stopped running the shardwidth22 tests in the move to
Gitlab, so when we re-enabled them we finally noticed that they were
broken in the client.
All this change does is takes the shardWidth value from the core
featurebase package instead of using a hardcoded value in the client package.
also use a single cluster with each test creating a different index
rather than each test creating a whole new cluster.
runtime went from 38s to 30s in my informal tests
discovered that client tests weren't running due to integration build
tag. Fixed the file I needed to get through SonarCloud and documented
rest of what needs to be done in FB-1152 https://molecula.atlassian.net/browse/FB-1152
fixes pathological case where imports with randomly ordered IDs which
spanned multiple shards and included ints or mutex fields could be
incredibly slow due to making 1000s of requests.
i used this script, a little clunky but it got the job done
```bash
for file in `find . -type f -print | grep '\.go'`; do
sed '1,/^\/\/ limitations under the License.$/d' $file > $file.tmp;
result=`cat $file.tmp`
if [[ result != "" ]]; then
gofmt $file.tmp &> /dev/null;
if [[ $? == 0 ]]; then
mv $file.tmp $file && gofmt -w $file;
else
rm $file.tmp;
fi
else
rm $file.tmp;
fi
done
```
