* add bsi base back to int value
* test bsi base/min/max for IntFields
motivated by bsi base not being added back to values
in extract calls when min was a positive integer.
* use mainline etcd-io dependencies, not forks
this commit does lots of things around clustering with goal of increasing stability.
- upgrades from molecula/etcd to go.etcd.io/etcd@v3.5.4
- upgrades from seebs/bbolt to go.etcd.io/bbolt@v1.3.6
- update tests to use unix sockets for etcd cluster communication
- this is what etcd uses for a lot of internal testing, so if their devs think
it's a valid test, we can probably accept that
- cleanup etcd node-watcher shutdown process
Co-authored-by: tgruben <tgruben@gmail.com>
* moved random query to another repo
it had weird dependency issues with upgrading to mainline etcd bc of the vegeta dep
so we removed it bc no one really uses it anyway
we got this error message:
```
github.com/molecula/featurebase/v3/cmd/random-query imports
github.com/tsenart/vegeta/v12/lib tested by
github.com/tsenart/vegeta/v12/lib.test imports
github.com/streadway/quantile tested by
github.com/streadway/quantile.test imports
.: "." is relative, but relative import paths are not supported in module mode
```
* add cleanup to EtcdUnixSocket test util
Co-authored-by: reesporte <reesedporter@gmail.com>
- terraform scripts to set up cluster
- cloud-formation scripts to set up cluster
- set up ingest node with kafka server and datagen
- set up second ingest node with molecula-consumer-kafka-static
- set up datadog in all nodes (ingest + featurebase)
- set up script to execute different queries
- only run delete test on schedule
previously, we would use the standard view if the query seemed to
cover all the views we had, or if we didn't seem to have any time
views. This is unintuitive if some views have been deleted (which
comes up a lot more often with TTL!). It's also unintuitive if you
know you haven't set any data w/ a timestamp and your query that
specifies a time range returns any data.
we have an issue where nodes get into UNKNOWN state rather frequently
during periods of high load when they aren't actually down. We want to
allow queries in this situation rather than giving a "shard
unavailable" message.
We default to the jmp-hash method which we had previously, and allow a
user to set the "modulus" option which uses a simple mod operation to
ensure an even spread of partitions across nodes.
I think that ideally we would have new indexes uses modulus and
existing indexes use jmp-hash which implies supporting this
configuration on a per-index basis.
If we don't do per index, we should probably run the whole test suite
both ways.
we change this to use a simple modulus to ensure maximally even
assignment of partitions to nodes rather than the hash thing we were
doing previously which may have helped minimize data movement when
adding nodes, though I'm not even sure of that.
The logic was duplicated in a few places, so we've also condensed
that. For now, we're skipping tests which have baked in assumptions
about which node a partition will end up on as we expect them to fail
until they are updated.
* 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>
Fixed the issue where deselecting variables didn't work on quries like
groupby and extract.
The empty variables list returns All() when the variables are empty.
you can get a context deadline exceeded from clientv3 if there's heavy load and the
etcd server sends a grpc code DeadlineExceeded. this causes the etcd client to not
retry connecting and then you end up with nodes that can't recover.
Co-authored-by: tgruben <tgruben@gmail.com>
Co-authored-by: 54mir <48686912+54mir@users.noreply.github.com>
Co-authored-by: seebs <seebs@molecula.com>
Co-authored-by: tgruben <tgruben@gmail.com>
Co-authored-by: 54mir <48686912+54mir@users.noreply.github.com>
Co-authored-by: seebs <seebs@molecula.com>
The intent of these lines was "if there's no filter, return
immediately rather than doing operations".
But actually we didn't write that, so we were calling intersectionCallback
on empty filters, which didn't matter since it failed out quickly, but
it's still a waste of effort.
Except we shouldn't get to these anyway because ConsiderKey already
correctly rejected these cases. I think. But still.
In fact, we *do* want to skip ahead sometimes to the next thing, and
only call our callback for things that match our filter. I was thinking
that we needed to call the callback for all data regardless, because
what if you're writing to a mutex and adding new data.
But even if you're adding new data, it's still in the filter, because
it has to be, because we don't start out knowing there's no existing
data. So the mutex actually works fine.
So the rule for BitmapBitmapTrimmer is that your filter doesn't have
any meaning other than (1) it tells us which containers you need
to see, (2) we provide it to your callback function. Maybe you want
to subtract those. Maybe you want to add them. That's up to you to
decide.
Now that we have ApplyRewriter, it's a viable way to implement ImportMutex.
It can be slower on low-density writes, because it's checking more things
than it otherwise might -- the other filter form can skip ahead and only
check the containers it's modfying, in principle, while this one doesn't
know it can do that. (The decision as to how far to skip ahead has to
be made in the BitmapBitmapTrimmer, while it's the callback provided to
it that knows when it next has data to write.)
On the other hand, it's probably faster in some cases, and would be
more-faster if we could improve the cursor management a bit, and it's
skipping at least some seeking because it doesn't need to use
ImportPositions after reading the whole thing.
This uses the shiny new ApplyRewriter logic for ClearRecords,
mostly to verify that ApplyRewriter works at all.
This also implies separating the cache update code out from
importPositions so it can be used also by this.
We also use fragment.ClearRecords instead of the different clearFragment
code in executor. The clearFragment implementation did not update TopN
caches and the like. Standardize it on the clearRecords implementation
which does.
We don't need to manually copy each individual item and keep
checking for the second index being out of bounds for every
item, we know it can't change at this point, so we can just bump
it over. We want this operation because BitmapRewriter can
use it to simplify trimming in some cases.
The filter and rewrite logic are unlocking and relocking but I don't
think they should. I think those locks were added early on during
testing of the filter stuff, but I don't think they should be needed,
and I've been unable to find a case where they were. I think probably
I had something where a ConsiderData function was trying to run a Tx.
This in a parallel to ApplyFilter/BitmapFilter which allows writebacks
while it's running. It's a write operation, so it needs a write lock
on the Tx, and needs to create bitmaps if they don't already exist.
The semantics are a bit messy and need better documentation still.
We frequently want to grab the set of values from a []uint64
that correspond to a given key, and make a container from them, but
sometimes we only want to do one of these. This implementation
lets us do that the same way every time, and do in-place
container creation without extra allocs.
The test for bitmap-to-array succeeding doesn't work with roaringParanoia,
which *does* intentionally panic at that point. Possibly we should also
drop the corresponding logic that tries to prevent it from panicing,
since it won't work with the paranoia flag on anyway.
