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
```
We were trying to write an error to a ResponseWriter After attempting to
write to it, and this produces messages about superfluous WriteHeaders,
which is correct. This patch changes things so that we report messages
more clearly and verbosely if we hit them before writing, and if we try
to write and fail, we log the message because that's all we can do.
This does change semantics slightly, in that now we're marshalling
separately from trying to write the marshalled data. I think this is
probably a reasonable call because it lets us get diagnostics about a
hypothetical encoding problem, but in practice I don't think there
should be any encoding problems. So my guess is the actual error will
occur in that last line, and be logged to the server console instead
of failing to write over HTTP.
Also note that this changes some of the messages to include the
underlying error they're complaining about.
We also merge the create/find and index/field cases because only a
couple of lines of code changed between four largeish functions,
and we test some of the failure cases.
We don't have test coverage on the "field isn't provided" type things
because the mux won't actually route things there without them, so
far as I know.
I'm not actually sold on this, but I'm not entirely unsold on it. It seems like
it does reduce the amount of duplication a lot, but also it's sort of a mess.
In the process, noticed that it makes more sense to grab the whole cluster
rather than just the nodes for an arbitrary shard for the shard==^0 case,
because then if we have an API (but no Qcx), we can be reasonably confident
that we'll be able to pick the local node for loopback even if we aren't
using the API directly.
Have thought about whether we should create our own Qcx in cases like that
but I really don't like the idea of automatically creating a Qcx.
Underlying goal: Don't use the http client to send messages back to the
local host. Also, when sending data to other nodes, don't collate it
from an ImportRequest into a completely different format, then immediately
collate that back into an ImportRequest. This does require changing
the logic over in ctl/import to make it create an ImportRequest.
Also, add additional testing to make sure we're actually trying anything
at all with several combinations (such as submitting import requests
which don't match the configuration of index or field), and improve
test coverage for that.
This introduces the ability to tell an http/client InternalClient about
a specific API that it should use for local queries where applicable.
That's not implemented outside of the import stuff, but should probably
be applied eventually to other things that are trying to talk to many
nodes one of which may be the local node. That behavior is contingent
on passing in a Qcx, because it is implicitly tied to an existing
execution context, and it can't assume that it can create a new one,
because that could deadlock.
We add endpoints and protobuf encode/decode to allow for sending
sharded requests over the wire in protobuf, so we can take our
sharded data and send it to other nodes if needed.
This is a squash of >15 other commits, so a bit of history
is relevant:
The Request type had FieldTypes in it because the field type
information was needed for sharding because sorting requires
that information. We change this around to make the external
sharding operation require the field types, and curry that
through the codec -- the codec is needed to tell the request
how it shards. (This is because the correct sorting order
varies by field type.) Requests (and ShardedRequests) no
longer have that table in them.
And then we hit a nasty bug in production and RCA showed
that our testing wasn't good enough and we need to be more
careful, and I discovered that test coverage in this package
was around 70%.
So, the other big thing here is coverage testing; in order to
make coverage testing viable and programmatically testable,
we have added the ability to render requests *back* to
JSON. This is not a great idea, but it does allow us to do
a lot of sanity-checking and verify that the encodings we're
using are consistent and correct.
This, plus some specific tests of decoding specific flawed
inputs, has caught a number of issues. Which are now fixed!
A lot of internal API surface got slightly changed, in ways
that make it simpler to work with. For instance, the
(*FieldOperation).TranslateUnsigned function doesn't really
need to exist; we can just have a non-method translate
function for unsigned and for signed, and use them based on
field type.
The stable translation hack used for testing had a bug that
could allow it to end up producing incorrect results if you
asked it to translate an ID first rather than exclusively
asking it to translate strings first, this has been
corrected. (This is a bug fix in code that was added
partway through creating this, but is tricky enough to
mention its own comment.)
Test coverage is now just over 90%, and a lot of what's left
is error-check returns that may well be actually unreachable
unless, say, the documentation for encoding/json is full of
lies. Which it probably is.
We support query parameters for details (default false) which
request additional data, and for a limit (default 0/MaxInt32)
on number of results returned to limit the amount of spam
produced if there's a lot of results. The simpler default
output should reduce load and runtime significantly, and the
ability to specify limits makes it easier to get reasonably
small responses.
There's some context support here, but the underlying filters
don't take contexts or check for them, which is probably
a flaw but might be a bit large to correct for this.
Despite being large, this set of changes is actually
fairly well contained within the mutex-checking code.
This implements a fairly straightforward sanity-check for mutexes,
implemented as a bitmapfilter at the fragment level, and with higher
levels combining results. There's two endpoints, an internal endpoint
which only checks the local node's shards, and an external one which
forwards requests (using the internal endpoint) to all the other nodes.
The internal endpoint does not do key translation, the external one
does.
The transmission format is a probably-inefficient JSON blob, and
returns data separated per-shard so we don't have as much merging
work to do.
This introduces a horrifying monstrosity function which tries to
sneakily corrupt mutex fields and which has to be exported (EWWWWW)
but which is only present in _test code (!??!! THIS WORKS WHY).
Also one typo fix in unrelated code caused by not wanting to keep
fighting with gofmt about this.
This is a design to let us write test cases for ingest with schema setup
and data in the json formats we want to use, and results as alternating
queries and expected results, so we can just create new test files and
run the tests against them. We also have to report back what we created
when creating things.
In the process of developing this, I noticed that the documentation describes
ingest schema as allowing more than one schema operation, but we didn't support
this, and also it wouldn't do much good because there was no way to do partial
things like "just add a field". Fixed.
Also we implement comparison for ops, so the test output is actually
a test rather than just some data to visually eyeball.
In the process, realize that the handling of timestamps was wrong; we said that we
take them as raw numbers relative to the epoch, not as raw Unix timestamps.
Also a couple of related cleanups caught by doing the testing.
This partially-implemented prototype of the ingest API is based on our
programmatic ingest API reference. It has noticable limitations, most
crucially that it doesn't handle multi-node clusters right now. However,
it basically implements the expected semantics.
There's some noticeable performance issues to do with the high overhead
of sorting bits in order to import them efficiently, but this is fixable.
We also add the hooks to the internal client, and make the finisher logic
a bit smarter.
Much of this code was originally by Nia Weiss, but it's been merged
and restructured a bit to get things broken into logical commits.
In fact, we have a number of things assuming that values passed to Import
always fit within a single known shard, so, drop all the extra complexity
around this, drop the computation of fancy view/shard keys, and so on.
There's a lot of room left to improve this probably but it's at least
better, I think.
Unfortunately, there's a handful of things, basically all of which are
test cases, which were relying on this, so, we also add functionality
for splitting import requests by shards. But this allows us to stop
duplicating each shard's inputs one at a time... which turns out to
mean that we now care that the import operation can write back to the
import request. This only affects test cases, so we adopt a crufty
hack involving cloning import requests in those rare cases, and also
when reusing the same column IDs to write to the existence field that
we'd be using later to write to another field.
Note that even if we weren't overwriting the column IDs with positions,
we'd be sorting the column/row ID lists by row-then-column, which means
we'd still be corrupting the column ID lists. This may want to change
at some point.
We also reuse a single Tx for all the views, because DB-per-shard
means that should work fine, and reduces the cost of doing these
updates, probably.
This works around an issue where unreplicated keys will not be matched everywhere.
This also avoids the cost of creating millions of bolt read transactions and allocating strings.
Attributes are unmaintained and unused.
They have become more of a liability than a benefit.
This change eliminates them from the codebase.
The only user-visible change (assuming that attrs are not used) is that the attrs field will no longer appear in row JSON.
This is used to handle a possible case where a kafka partition is moved to another ingester while a previous ingester is still processing it, causing 2 ingesters to process it at the same time.
This allows a duplicate ingester to skip past messages which have already been ingested.
The functional option and returned closure combine to result in
us using the same sync.Mutex object for every TranslateReader on
a given server, which means that if one of them isn't producing anything,
we eventually end up waiting on that with all the others blocked
waiting for the lock. Use separate locks for each, of the same
type as the one initially provided as a template. This does mean
that multiple readers can be operating at once, but in theory
no two readers should ever be writing to the same stores, we
think.
