Commit graph

25 commits

Author SHA1 Message Date
tgruben
0aa5efcc51
staticcheck fixes (#2278) 2022-11-07 10:51:55 -06:00
Jacob Brinlee
dd30168b1c
updating topic/partition/offset order (#2250)
* updating order of topic/partition/offset in log message
2022-11-03 09:20:49 -05:00
Travis Turner
00ef2380e5
Batch insert via SQL (multiple tuples) (#2243)
* 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>
2022-10-29 14:24:33 -05:00
Pranitha-malae
ccd2e7ea63
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
2022-10-20 16:51:21 -05:00
Pranitha-malae
09baf99ce4
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>
2022-10-19 12:16:38 -05:00
Travis Turner
800750c746
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.
2022-10-14 14:02:36 -05:00
jonathan.cheng
43f63b93e3
Cloud 940, skip bad row, enable ingest subsequent rows (#2201)
* first try to skip decodeMessage() error

* force idk to skip a row if there are errors in recordizing

* add comment for removing returning errors from decodemessage()

* added ingest flag SkipBadRows and implementation for skipping Bad Rows (errors that come from recordizer)

* added some comments

* removed comment

* made changes as per discussion with jaffee and walter. i hope this works...

* adding unit tests to check functionality implemented for CLOUD-940

* addressing review comments

* changed a variable name in test file

* removed a variable from ingest test file

* testing sonarcloud failure

* drop spurious second sonar-scanner call

We call sonar-scanner on the IDK data, and then we change
into the IDK directory and try to run it again on the same files,
which don't exist.

* abandon idk change detection for now

the "changes" rule appears not to be good at detecting changes
in some cases. specifically, it appears that you have to be in
an "only:" clause, not a "rules" clause, to trigger the
merge-specific behavior which checks the entire merge branch
instead of the top commit, but that means that if your last
commit doesn't touch IDK, we don't run IDK tests, and I haven't
been able to fix this yet.

So for now, revert the IDK-specific change detection behavior,
which slows CI down but gets us test coverage.

* fix path references

we had three tests all creating idk_coverage.out, then we tried
to grab all files named coverage.out from the testdata directory.

* refactoring tests to avoid duplication

* reverting changes made for local testing

Co-authored-by: CHIN JUNG CHENG <chengcj@CHINs-MacBook-Pro.local>
Co-authored-by: Pranitha-malae <56414132+Pranitha-malae@users.noreply.github.com>
Co-authored-by: Pranitha-malae <pranitha453@gmail.com>
Co-authored-by: Seebs <seebs@molecula.com>
2022-10-06 16:20:35 -05:00
Hoang Pham
634c54e257 FB-1696 - fixed debug message to show topic's name instead of topic's address 2022-10-04 15:50:46 -05:00
HHans09
5a66d283f0 FB 1646 : Code updated to make the debug message more helpful 2022-09-26 16:32:19 -04:00
HHans09
3bb825a908 FB-1646 : UPdated code post code review 2022-09-26 16:32:19 -04:00
HHans09
555c8f6532 FB-1646 Removed Debugf messages that does not make sense 2022-09-26 16:32:19 -04:00
HHans09
d19b7ac841 UPdated the code to remove declared but unused variables - committedOffsets, stv & iv 2022-09-26 16:32:19 -04:00
HHans09
80281edaee Removed Debugf messages that does not make sense 2022-09-26 16:32:19 -04:00
Seebs
c3b032d5cb drop ioutil
The ioutil package is deprecated, with all of its functions having
moved into os or io. Do the replacements so we stop having this
impending.
2022-09-23 16:56:27 -05:00
rachithrr
8b74087c5c
FB-1674: Kafka consumer stops reading messages from topic (#2222)
Added condition to first sort by topic, followed by partition and offset.
2022-09-21 12:53:50 -05:00
Garrison Davis
6562b60340
Fix idk Version and idk BuildTime (#2205)
When we merged idk into featurebase, we didn't change the import path in
the Makefile to point to the correct go path
2022-08-25 17:43:13 -06:00
Garrison Davis
123b20d2f9
Upgrade Go to 1.19 (#2204)
Upgrade Go to 1.19

* Use go install to install statik for CI/CD

* Switch from stretch to buster for idk

The stretch release doesn't exist anymore for go 1.19 docker images.

buster is a newer version of Debian anyway (v10 vs v9)

Co-authored-by: Fletcher Haynes <fletcher.haynes@molecula.com>
2022-08-25 14:54:14 -06:00
Garrison Davis
9eb6b3ac3f
Fix idk package_docker_all CI job (#2196)
Before, we were building docker images for IDK for each of the four
linux/darwin amd64/arm64 platfrom/arch combinations, which didn't make
sense. If we want to later build docker images for linux/arm64, we can
add that later.

I also cleaned up the Dockerfile for IDK to minimize creation of excess
layers (by &&-ing RUN commands), and made apt quieter to cut back some
of the noise.
2022-08-18 13:52:04 -06:00
Travis Turner
cddd9871af
Distinguish between three copies of fakeidp service (#2192)
There are currently three copies of a package called `fakeidp` in the
featurebase repo:

- ./idk/fakeidp/go.mod
- ./internal/clustertests/fakeidp/go.mod
- ./qa/fakeidp/go.mod

All three have a `go.mod` file. While this is supported under golang's
new Workspace support, what's not supported is that the modules share
the same name (in this case "fakeidp"). This commit is a sort of
temporary fix which renames the module for two of the instances. This
prevents, for example, VSCode with workspace support enabled, from
barfing.

By the way, one can enable VSCode workspace support with the following
setting:

```
// gopls
"gopls": {
    "build.experimentalWorkspaceModule": true
}
```

Also...

This commit fixes the `make testv` target. It's probably not used
anywhere (which I'm assuming because it was broken), but it's a handle
target, so now it will list and run tests against all packages found in
the repo, including the root package.
2022-08-16 09:13:11 -05:00
wmanongdo
baab212bb1
[CLOUD-934] Optionally broadcast IDK Kinesis errors/panics to externa… (#2185)
* [CLOUD-934] Optionally broadcast IDK Kinesis errors/panics to external storage

- Add a minor public method `idk.Main.SetLog` to allow setting the logger instance
  after initialization.
- Add a Logger implementation that captures recoverable errors and panics
  and pushes to an external store. Meant to decorate an existing Logger
  instance and always delegate to its implementation. Decoration happens
  when all AWS resources are initialized. Before then, the wrapped Logger
  implementation is used.
- If `--error-queue-name/CONSUMER_ERROR_QUEUE_NAME` specified, use an
  ErrorStreamLogger to push errors and panics to an SQS queue with that name.
  Omission of the option preserves current behavior.
- Parse sink ID from the `--stream-name/CONSUMER_STREAM_NAME` expecting the form
  'PREFIX'-VALID_UUID. If the sink UUID is invalid, emit a warning that errors/panics
  will not be written to an SQS queue but will still be logged using the decorated
  Logger instance.
- The inability to push to an SQS queue leads to warnings being emitted to notify
  ECS that no queue will be written to and is NOT a hard error.
- Add SQS interface mock for unit testing.
- Add IDK make targets for generating mock interfaces.

* [CLOUD-934] Execute go mod tidy and go fmt to pass CI/CD checks

* [CLOUD-934] Remove extraneous Makefile in idk/kinesis and fix install-mock-generator target

* [CLOUD-934] Add godocs to exported types and functions

* [CLOUD-934] Changed warning to not sound so ominous and update associated unit test

* [CLOUD-934] Unblock CI/CD at the IDK test stage
2022-08-12 17:59:56 -05:00
souhailanoor
39def696c5
FB-1618: Fix code smells due to unifying IDK and Featurebase (#2184)
* create constants for duplicates to resolve code smells

* refactored duplicated code for extract(rows))

* additional refactoring

* refactor test

* rename to match reg exp

* fixed naming

* fixed naming, removed duplicated string

* fixed index names

* fixed bug introduced in earlier commit

* create delete index method and refactor if statement

* refactored file to reduce complexity

* fix go fmt error

* fixed bug introduced in last commit

* address reviewer's comments

* remove trailing colon and spaces
2022-08-12 17:00:29 -05:00
tgruben
7cfa516f74
[FB-1617] Commit Messages out of order (#2180)
* commiting partition highmark offset only; 
* protected spool access with lock
2022-08-08 14:41:22 -05:00
Samir Patel
3681feeeb2
[FB-1024 FB-1590] Increase timerange (un-revert) (#2174)
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.
2022-08-04 15:20:45 -07:00
tgruben
7a53e1e830
update to 1.9.1;make rm safe (#2175) 2022-08-02 15:09:20 -05:00
souhailanoor
2bdf1f5081
FB-1597: unifying idk and featurebase (#2160)
* unifying idk and featurebase: first pass

* resolved conflict with master for gitignore & dockerignore

* deleted binaries that were accidentally pushed to git

* combined gitlab jobs for idk & featurebase

* run go fmt for idk

* updated ssh env variable, and made docker password variable in gitlab env variables

* fixed typo assigning variable name

* trying to fix docker login error

* trying a different solution for docker password

* pass registry

* fixed docker login

* updated paths for idk

* exclude idk tests from featurebase test run

* fix vendor error

* update certificates

* grpc needs to be in version 1.38
genproto, which is imported by big query updates the grpc version to 1.47.0
grpc 1.47.0 causes etcd to deadlock when calling etcd.Close()
the fix is to have a replace in go.mod to specify a specific grpc version

* run go mod tidy

* go mod

* run go mod tidy

* exclude bigquery since it is causing issues and undo grpc replace in go.mod

* fix grpc version

* fix formatting error

* update formatting

* attempt to fix formatting

* update path for code coverage

* update to use current branch binaries, not master

* fix for building idk - path updates

* udpate path for binaries

* update job dependecies

* update docker idk tests to use the current branch registry

* update stages for jobs

* updated job dependencies

* not allow idk s3 dump to fail since it is a dependency for integration tests

* update dependecy for idk tests

* update paths for idk build and code coverage

* download featurebase binary from s3

* pass branch name to all setup scripts

* change to current branch instead of master

* updated sonarcloud

* sonarcloud fix and branch name fix

* trying to speed up pipeline run time

* update stage

* branch name fix + sonar cloud

* sonarcloud
2022-07-28 17:23:16 -05:00