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.
We've been seeing weird retention of Tx that shouldn't still be open, and
one possible explanation is that, until a Tx actually uses the freelist
cursor (either to allocate a page or to release it back to the freelist),
the freelistCursor statically stored in the Db object continues to have a
pointer to the previous Tx which used it, which allows a Tx, and thus its
dirty page map, to be retained forever.
I previously thought this should also nil out the page maps in the Tx, but
the more I think about it, the less I think that's a good idea. The actual
lifespan of a committed Tx should be quite short. If it *does* stick around,
it's beneficial to us as debuggers to see those large maps of dirty pages
sticking around. So after thinking about it a lot I decided not to do
that.
Similarly, when closing out a container filter (whether a filter or
a rewriter), zero out the Cursor, Tx, and filter and rewriter functions.
(We don't have to worry about the cursor's Tx, because the cursor gets
closed, which zeros its Tx and returns the cursor to the cursor pool,
too.) This likely matters a lot less, as the filters in the pool
get garbage collected "soon", but it still reduces the amount of
stuff being retained.
* make CookieName an exported constant
* fix SetGRPCMetadata
this will actually set the grpc metadata even if there are no cookies in the
metadata already.
* gofmt yourself
* FB- 1456 - TTL - fixed views not returning correct results when least precise quantum are deleted
* FB-1456 - TTL - PR - fixed comment
* FB-1456 - TTL - changed getQuantum to getLowestGranularityQuantum since we only care about the least precise quantum that is available
* FB-1456 - TTL - removed unit test used for debug
* FB-1456 - TTL - fixed comments
* [FB-1435] BSI Base Fix (#2056)
* 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.
* FB-1456 - TTL - fixed comments
Co-authored-by: Samir Patel <48686912+54mir@users.noreply.github.com>
* 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.
