Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(ci): resolve send coverage errors #1849

Closed
wants to merge 78 commits into from
Closed

Conversation

joshua-goldstein
Copy link
Contributor

Problem

CI is unable to send coverage results to coveralls.io.

darkn3rd and others added 30 commits April 15, 2021 08:28
zstd is not set by default even when cgo is enabled.
Add a Builder type in skiplist package which can be used to insert
sorted keys efficiently. Add a test and benchmark for it.
This change makes the skiplist grow for the case of sorted 
skiplist builder. The normal skiplist still cannot grow. 
Note: The growing skiplist is not thread safe.

Co-authored-by: Ahsan Barkati <ahsanbarkati@gmail.com>
In Dgraph, we already use Raft write-ahead log. Also, when we commit transactions, we update tens of thousands of keys in one go. To optimize this write path, this PR introduces a way to directly hand over Skiplist to Badger, short circuiting Badger's Value Log and WAL.

This feature allows Dgraph to generate Skiplists while processing mutations and just hand them over to Badger during commits. It also accepts a callback which can be run when Skiplist is written to disk. This is useful for determining when to create a snapshot in Dgraph.
…isher (#1697)

When a skip-list is handed over to badger we should also send the
entries in skiplist to the publisher so that all the subscribers get notified.
This PR adds DropPrefixNonBlocking and DropPrefixBlocking API that can be used to logically delete the data for specified prefixes.
DropPrefix now makes decision based on badger option AllowStopTheWorld whose default is to use DropPrefixBlocking.
With DropPrefixNonBlocking the data would not be cleared from the LSM tree immediately. It would be deleted eventually through compactions.

Co-authored-by: Rohan Prasad <prasad.rohan93@gmail.com>
Add benchmark tool for picktable benchmarking.
#1700)

This PR adds FullCopy option in Stream. This allows sending the table entirely to the writer. If this option is set to true we directly copy over the tables from the last 2 levels. This option increases the stream speed while also lowering the memory consumption on the DB that is streaming the KVs.
For 71GB, compressed and encrypted DB we observed 3x improvement in speed. The DB contained ~65GB in the last 2 levels while remaining in the above levels.

To use this option, the following options should be set in Stream.

stream.KeyToList = nil
stream.ChooseKey = nil
stream.SinceTs = 0
db.managedTxns = true

If we use stream writer for receiving the KVs, the encryption mode has to be the same in sender and receiver. This will restrict db.StreamDB() to use the same encryption mode in both input and output DB. Added TODO for allowing different encryption modes.
Remove "GitHub issues" reference. (we use discuss now)
Remove Datadog's ZSTD that requires CGO
Make Klauspost's ZSTD as default
This PR adds support for stream writing incrementally to the DB.
Adds an API: StreamWriter.PrepareIncremental

Co-authored-by: Manish R Jain <manish@dgraph.io>
While doing an incremental stream write, we should look at the first level on which there is no data. Earlier, due to a bug we were writing to a level that already has some tables.
I propose this simple fix for detecting conflicts in managed mode. Addresses https://discuss.dgraph.io/t/fatal-error-when-writing-conflicting-keys-in-managed-mode/14784.

When a write conflict exists for a managed DB, an internal assert can fail.
This occurs because a detected conflict is indicated with commitTs of 0, but handling the error is skipped for managed DB instances.

Rather than conflate conflict detection with a timestamp of 0, it can be indicated with another return value from hasConflict.
)

With the introduction of SinceTs, a bug was introduced #1653 that skips the pending entries.
The default value of SinceTs is zero. And for the transaction made at readTs 0, the pending entries have version set to 0. So they were also getting skipped.
* According to CHANGELOG.md#removed-apis. `TableLoadingMode` option is removed.
fixup the memory usage part of doc.

* Remove `Options.ValueLogLoadingMode` part.
joshua-goldstein and others added 13 commits October 13, 2022 20:30
## Problem

Linter was not running due to Go config issue. See issue here
#1810.

## Solution

We refactor the setup for the linter. See relevant discussion here
golangci/golangci-lint#1920. Go setup for
linter is now on parity with the badger test CI workflow.
## Problem
 
Errcheck linter is failing.

## Solution

On [Dgraph](https://github.com/dgraph-io/dgraph/blob/main/.golangci.yml)
and
[Ristretto](https://github.com/dgraph-io/ristretto/blob/main/.golangci.yml)
repositories, neither is running errcheck. We temporarily disable
errcheck here to bring this repository to parity. We also do some
cleanup in the Readme.
## Problem

We are not reporting test coverage.

## Solution

We show test coverage with coveralls.
## Problem

Previously we were running a nightly Badger bank test (4 hours).

## Solution

We bring back the nightly badger bank test.
## Problem

Resolving more lint errors.

## Solution

We fix some basic gosimple and gofmt lint errors. We raise the lll line
length to 120 (as in Dgraph repo).
## Problem

Lint tests were failing.

## Solution
 
We comment out varcheck and gosec.  We will resolve these later.
## Problem

gas linter was running gosec, which we will resolve later.

## Solution

We comment out gas linter.
## Problem

CI jobs are not running when updates are made to release branch.

## Solution

Run all CI jobs when PR's are opened against release branch.
This PR adds CD steps for Badger releases. Artifacts (badger binary and
checksum) will be uploaded automatically to Github. Final step will be
to add artifacts to release. This reflects the process we already have
in place for Dgraph.

Badger build flags were taken from the [Dgraph release
script](https://github.com/dgraph-io/dgraph/blob/main/contrib/release.sh).
We add a Makefile to streamline the build process.

(cherry picked from commit 11c81e3)

## Remark

PR is duplicate (cherry-pick) because we have two branches running in
parallel (main and release/v3.2103).
Latest runner tag now uses ubuntu-22.04.  We pin to ubuntu 20.04.
## Problem

Currently we only deploy amd64 badger CLI tool builds. We would like
arm64 builds too.

## Solution

Use an arm64 self-hosted runner to build arm64 badger CLI tool.
Currently [appveyor
tests](https://ci.appveyor.com/project/manishrjain/badger/builds/42502297)
are failing in multiple places on Windows.

Original PR #1775

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg"
height="34" align="absmiddle"

alt="Reviewable"/>](https://reviewable.io/reviews/dgraph-io/badger/1775)
<!-- Reviewable:end -->

<!--
 Change Github PR Title 

 Your title must be in the following format: 
 - `topic(Area): Feature`
- `Topic` must be one of
`build|ci|docs|feat|fix|perf|refactor|chore|test`

 Sample Titles:
 - `feat(Enterprise)`: Backups can now get credentials from IAM
 - `fix(Query)`: Skipping floats that cannot be Marshalled in JSON
 - `perf: [Breaking]` json encoding is now 35% faster if SIMD is present
 - `chore`: all chores/tests will be excluded from the CHANGELOG
 -->

## Problem
 <!--
 Please add a description with these things:
 1. Explain the problem by providing a good description.
 2. If it fixes any GitHub issues, say "Fixes #GitHubIssue".
 3. If it corresponds to a Jira issue, say "Fixes DGRAPH-###".
4. If this is a breaking change, please prefix `[Breaking]` in the
title. In the description, please put a note with exactly who these
changes are breaking for.
 -->

## Solution
 <!--
 Please add a description with these things:
 1. Explain the solution to make it easier to review the PR.
2. Make it easier for the reviewer by describing complex sections with
comments.
 -->

Co-authored-by: Alexey Ivanov <SaveTheRbtz@GMail.com>
Co-authored-by: Joshua Goldstein <92491720+joshua-goldstein@users.noreply.github.com>
## Problem
We don't need an `apt upgrade`* step here. this is an overkill - plus
it's causing some dependency issue on the environment layer. it's also
causing a bunch of tests to fail in the installation step -
https://github.com/dgraph-io/badger/actions .. we don't have a
consistent baseline mostly because of the failing environment setup
step.

we should have a tight pinning on our environments. operations like
`upgrade` etc. can induce failures suddenly based on upstream changes.

issue 
-
https://github.com/dgraph-io/badger/actions/runs/3909870547/jobs/6681427503
- https://github.com/dgraph-io/badger/actions?query=event%3Aschedule

## Solution
rm upgrade*
skrdgraph
skrdgraph previously approved these changes Jan 24, 2023
@skrdgraph
Copy link
Contributor

seems to have failed ... because of the same issue. @joshua-goldstein - I am not sure what's causing this:

422 Unprocessable Entity response status code indicates that the server understands the content type of the request entity, and the syntax of the request entity is correct, but it was unable to process the contained instructions.

I am thinking of regenerating this token.

@joshua-goldstein
Copy link
Contributor Author

We can try that. Looks like it is a generic error message. Looks like others have had similar issues:
lemurheavy/coveralls-public#632
lemurheavy/coveralls-public#1264
lemurheavy/coveralls-public#1268

@skrdgraph
Copy link
Contributor

Looks like we really need to debug this failure now. We have tried changing the coveralls token as potential fix.

@mangalaman93 mangalaman93 changed the base branch from main-deprecated to main February 24, 2023 19:41
@mangalaman93 mangalaman93 dismissed skrdgraph’s stale review February 24, 2023 19:41

The base branch was changed.

@mangalaman93 mangalaman93 changed the base branch from main-deprecated-v4 to main March 1, 2023 07:16
@mangalaman93 mangalaman93 deleted the joshua/update-ci branch March 1, 2023 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.