fix duplicate pipeline versions #9966
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The det helm values are in sync now up to commit `d5f807da0ecded7d7b4c6afbf37bbfce8d0233f2`. They also include changes that have yet to be merged into main but are expected to be released in determined `0.31.0` --------- Co-authored-by: Jose Molina-Melendez <jose.molina-melendez@hpe.com> Co-authored-by: molinamelendezj <molinamelendezj@gmail.com>
Fixes a whitespace issue and will now work with Alphas and RCs
This PR refactors the backend to remove server MountedRepo state in favor of client state. It maintains existing behavior, updates all relevant tests, and adds new tests for PollMount.
This replaces an inflexible parser that's 1 line of code with a more complicated one that can parse any valid metadata. I changed the "replace" metadata op to `replace` in the CLI. `set` is short, but I can never remember it. The grammar looks like this:  `add` and `edit` take a `KV`. `replace` takes a `KVs`. Double-quoted strings are the same as Go's, except that `\U12345678` syntax isn't supported because the spec doesn't explain how it works and I don't know how it works. Single-quoted strings are very simple; they can't have any escapes in them. Our variant of JSON is more liberal than the official JSON spec out of convenience. You can now have vertical linefeeds and `\x??` in your JSON. You can also have a trailing comma. Finally, you can probably write `{"key":"value""key2":"value2"}`. I have fuzz tested the parser for about 12 hours x 32 cores, because it will panic when the parser selects a path that causes the lexer to not advance. These are mostly caused by `@@*` ("pick 0 or more of this big subobject") directives; that is why an entirely empty KVs is handled by checking that `strings.TrimSpaces() == ""`. Allowing empty KVs in the grammar is hard to get right without lookahead (which makes error messages bad); lookahead is especially bad for this grammar because how many tokens you need to look ahead depends on the exact content of double-quoted strings (because `(escape | double quoted chars)*` is the content of the string and different strings have different numbers of tokens). So we avoid lookahead entirely and deal with requring some valid data to parse. The JSON parser is also not the ideal implementation which is why quirks like omitting commas works. Frankly, JSON made its grammar harder to implement than it needed to be to make it more restrictive than it needed to be. With all that in mind, although we allow a bunch of weird stuff, we don't reject or misparse anything valid, so I'm happy.
Updates the helm deployment templates of `pachd` and `pachw` to require `pachd.storage.storageURL` be set if `pachd.storage.gocdkEnabled` is true --------- Co-authored-by: Fahad Syed <deadbeamNetworks@gmail.com>
Based on this notion doc https://www.notion.so/Copy-Pachyderm-File-URI-from-Jupyterlab-File-Browser-192a4a8a108548759cc197a010ab94cd It it is pretty similar to the doc, except I forgot I need to add a case that handles lack of clipboard permissions (due to lack of https). There isn't really a great way to work around this so it seems like just asking the users to copy the text from the notification is a good option. ## With Clipboard permissions <img width="353" alt="Screenshot 2024-04-21 at 8 49 29 PM" src="https://github.com/pachyderm/pachyderm/assets/401518/fff7b197-b55d-4131-a3ee-3a5ffad5b965"> ## Without Clipboard permissions (only users outside an https context will see this) <img width="365" alt="Screenshot 2024-04-21 at 8 48 50 PM" src="https://github.com/pachyderm/pachyderm/assets/401518/93aa6895-3834-4012-9ade-c420b49ea64e">
jrockway
reviewed
Apr 25, 2024
| FROM collections.pipelines a | ||
| INNER JOIN collections.pipelines b | ||
| WHERE a.name = b.name | ||
| AND a.version = b.version |
There was a problem hiding this comment.
This version is the version of Pachyderm that the stored proto is from, not the version of the pipeline.
| var createUniqueIndex = "CREATE UNIQUE INDEX pip_version_idx ON collections.pipelines(name,version)" | ||
|
|
||
| // collect all pipeline versions for pipelines with duplicate versions | ||
| var duplicatePipelinesQuery = ` |
There was a problem hiding this comment.
I think this query wants to be select count(1),array_concat(key) from collections.pipelines group by idx_version having count(1) > 1 or similar (find all idx_version values that have more than one row).
| pipValues = pipValues[:len(pipValues)-1] | ||
| stmt := fmt.Sprintf(` | ||
| UPDATE collections.pipelines AS p SET | ||
| p.version = v.version, |
#9849 broke this, fixing it.
The helm chart now deletes storage configuration if deployTarget=LOCAL. So, use CUSTOM.
This is a first pass at instrumenting the storage layer. There's a few
high level changes you'll generally see throughout this PR:
1. Child contexts have been passed throughout. Particular areas of focus
are distributed systems like the task service and low level constructs
like file set, index, and chunk readers & writers.
2. Metrics have been added. The metrics vary based on what is relevant,
but generally we're reporting bytes read/written as well as indicators
like cache misses vs cache hits, indices skipped vs indices read, and so
forth.
I'm sure I've missed areas that we want to instrument, but that's fine.
We can add those in a follow up PR.
Metrics are visible when running at a 'debug' level.
Here are some snippets of metrics:
```json
{
"severity": "debug",
"time": "2024-04-18T19:30:24.935036734Z",
"logger": "grpc.pfs_v2.API/ModifyFile.fileSetWriter.indexWriter(deletive-index-writer)",
"caller": "index/writer.go:85",
"message": "meter: bytes",
"service": "pfs_v2.API",
"method": "ModifyFile",
"command": [
"pachctl put file images@master -f liberty.jpg"
],
"peer": "127.0.0.1:48528",
"type": "int",
"delta": 35,
"meter": "bytes"
}
{
"severity": "debug",
"time": "2024-04-18T19:30:24.935196695Z",
"logger": "grpc.pfs_v2.API/ModifyFile.fileSetWriter.batcher(chunk-batcher).taskChain.upload",
"caller": "chunk/uploader.go:186",
"message": "meter: tx_bytes",
"service": "pfs_v2.API",
"method": "ModifyFile",
"command": [
"pachctl put file images@master -f liberty.jpg"
],
"peer": "127.0.0.1:48528",
"type": "int",
"delta": 133858,
"meter": "tx_bytes"
}
{
"severity": "debug",
"time": "2024-04-18T19:30:24.954077375Z",
"logger": "grpc.pfs_v2.API/ModifyFile.fileSetWriter.indexWriter(additive-batched-index-writer)",
"caller": "index/writer.go:85",
"message": "meter: bytes",
"service": "pfs_v2.API",
"method": "ModifyFile",
"command": [
"pachctl put file images@master -f liberty.jpg"
],
"peer": "127.0.0.1:48528",
"type": "int",
"delta": 155,
"meter": "bytes"
}
```
This reworks the collections package to avoid embedded contexts. This is especially important for ReadWrite transactions, which didn't use contexts at all. This resulted in many operations, especially related to auth, blocking indefinitely when Postgres was broken. That's now fixed. This replaces PR #9947. Part of CORE-2246
…incompatible (#9710) Bumps [github.com/docker/docker](https://github.com/docker/docker) from 20.10.17+incompatible to 20.10.27+incompatible. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/docker/docker/releases">github.com/docker/docker's releases</a>.</em></p> <blockquote> <h2>v20.10.27</h2> <p>For a full list of pull requests and changes in this release, refer to the relevant GitHub milestones:</p> <ul> <li><a href="https://github.com/docker/cli/issues?q=is%3Aclosed+milestone%3A20.10.27">docker/cli, 20.10.27 milestone</a></li> <li><a href="https://github.com/moby/moby/issues?q=is%3Aclosed+milestone%3A20.10.27">moby/moby, 20.10.27 milestone</a></li> </ul> <h3>Bug Fixes and Enhancements</h3> <ul> <li>Fix dockerd-rootless-setuptools.sh when user name contains a backslash. <a href="https://redirect.github.com/moby/moby/pull/46424">moby/moby#46424</a></li> <li>Add <code>IP_NF_MANGLE</code> to check-config.sh to the "generally required" list in check-config.sh because it is required by Swarm. <a href="https://redirect.github.com/moby/moby/pull/46674">moby/moby#46674</a></li> <li>Fix a deadlock in libnetwork which could prevent containers from starting. <a href="https://redirect.github.com/moby/moby/pull/46693">moby/moby#46693</a></li> <li>Write overlay2 layer metadata atomically. <a href="https://redirect.github.com/moby/moby/pull/46705">moby/moby#46705</a></li> <li>Support building with Go 1.20. <a href="https://redirect.github.com/moby/moby/pull/46694">moby/moby#46694</a> <a href="https://redirect.github.com/moby/moby/pull/46695">moby/moby#46695</a> <a href="https://redirect.github.com/moby/moby/pull/46696">moby/moby#46696</a></li> </ul> <h3>Packaging Updates</h3> <ul> <li>Update to go1.20.10, golang/org/x/net v0.17.0. <a href="https://redirect.github.com/moby/moby/pull/46692">moby/moby#46692</a></li> </ul> <h3>Security</h3> <ul> <li>Deny containers access to <code>/sys/devices/virtual/powercap</code> by default. This change hardens against <a href="https://scout.docker.com/v/CVE-2020-8694">CVE-2020-8694</a>, <a href="https://scout.docker.com/v/CVE-2020-8695">CVE-2020-8695</a>, and <a href="https://scout.docker.com/v/CVE-2020-12912">CVE-2020-12912</a>, and an attack known as <a href="https://platypusattack.com/">the PLATYPUS attack</a>. For more details, see <a href="https://github.com/moby/moby/security/advisories/GHSA-jq35-85cj-fj4p">advisory</a>, <a href="https://github.com/moby/moby/commit/81ebe71275768629689a23bc3bca34b3b374a6a6">commit</a>.</li> </ul> <h2>v20.10.26</h2> <h2>20.10.26</h2> <p>For a full list of pull requests and changes in this release, refer to the relevant GitHub milestones:</p> <ul> <li><a href="https://github.com/docker/cli/issues?q=is%3Aclosed+milestone%3A20.10.26">docker/cli, 20.10.26 milestone</a></li> <li><a href="https://github.com/moby/moby/issues?q=is%3Aclosed+milestone%3A20.10.26">moby/moby, 20.10.26 milestone</a></li> </ul> <h3>Bug Fixes and Enhancements</h3> <ul> <li>Support filesystems which do not support extended file attributes with the VFS graph driver. <a href="https://redirect.github.com/moby/moby/pull/45466">moby/moby#45466</a></li> <li>Fix AppArmor profile docker-default <code>/proc/sys</code> rule. <a href="https://redirect.github.com/moby/moby/pull/45716">moby/moby#45716</a></li> <li>seccomp: always allow <code>name_to_handle_at(2)</code>. <a href="https://redirect.github.com/moby/moby/pull/45835">moby/moby#45835</a></li> <li>Fix an issue which prevented volumes mounted to a live-restored container from being removed. <a href="https://redirect.github.com/moby/moby/pull/45840">moby/moby#45840</a></li> <li>client: resolve an incompatibility with Go 1.20.6, Go 1.20.7, Go 1.19.11 and Go 1.19.12. <a href="https://redirect.github.com/moby/moby/pull/45972">moby/moby#45972</a></li> <li>windows: fix <code>--register-service</code> when executed from within binary directory. <a href="https://redirect.github.com/moby/moby/pull/46217">moby/moby#46217</a></li> </ul> <h3>Packaging Updates</h3> <ul> <li>Update Go to 1.19.12. <a href="https://redirect.github.com/moby/moby/pull/46142">moby/moby#46142</a></li> <li>Update containerd to v1.6.22. <a href="https://redirect.github.com/moby/moby/pull/46105">moby/moby#46105</a></li> <li>Update runc to v1.1.8. <a href="https://redirect.github.com/moby/moby/pull/46031">moby/moby#46031</a></li> <li>Delete Upstart init scripts and clean up sysvinit. <a href="https://redirect.github.com/moby/moby/pull/46047">moby/moby#46047</a></li> </ul> <h2>v20.10.25</h2> <h2>Bug fixes and enhancements</h2> <ul> <li>Fix log loss with the AWSLogs log driver <a href="https://redirect.github.com/moby/moby/pull/45349">moby/moby#45349</a></li> </ul> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/moby/moby/commit/81ebe71275768629689a23bc3bca34b3b374a6a6"><code>81ebe71</code></a> Merge pull request from GHSA-jq35-85cj-fj4p</li> <li><a href="https://github.com/moby/moby/commit/fb636657a7a322bd5da4270c4d95f037576570b3"><code>fb63665</code></a> Merge pull request <a href="https://redirect.github.com/docker/docker/issues/46705">#46705</a> from thaJeztah/20.10_backport_atomic-layer-data-write</li> <li><a href="https://github.com/moby/moby/commit/b967d897586ca3e3e8dcd96d9b5712b5ac02bc9f"><code>b967d89</code></a> Merge pull request <a href="https://redirect.github.com/docker/docker/issues/46692">#46692</a> from corhere/backport-20.10/update-x-net-v0.17</li> <li><a href="https://github.com/moby/moby/commit/2c22bd52808f1c15d8efcdfb8c960f778c439a37"><code>2c22bd5</code></a> vendor: golang.org/x/net v0.17.0</li> <li><a href="https://github.com/moby/moby/commit/d862c21eb2a806ab72069664f6ba5a75006fc843"><code>d862c21</code></a> Update to go1.20.10</li> <li><a href="https://github.com/moby/moby/commit/cb47414f4187aff09c3d49ac93f7a15411c49d11"><code>cb47414</code></a> Merge pull request <a href="https://redirect.github.com/docker/docker/issues/46696">#46696</a> from corhere/backport-20.10/go1.20-enablement</li> <li><a href="https://github.com/moby/moby/commit/ea4eb7398c8adea7610ea9b0123cf19dad9a7817"><code>ea4eb73</code></a> Merge pull request <a href="https://redirect.github.com/docker/docker/issues/46695">#46695</a> from corhere/backport-20.10/safer-fileinfo</li> <li><a href="https://github.com/moby/moby/commit/6c523aabaec55e36c0b53f4b03844b377869ce91"><code>6c523aa</code></a> hack: fix suppressing Xattrs lint errors</li> <li><a href="https://github.com/moby/moby/commit/31b837499c2cd62bdd3d4cdfeaf83903dbc27f6b"><code>31b8374</code></a> pkg/archive: audit gosec file-traversal lints</li> <li><a href="https://github.com/moby/moby/commit/8e4485536ba493508afd894beb9f7c2bdbf4df99"><code>8e44855</code></a> Remove local fork of archive/tar package</li> <li>Additional commits viewable in <a href="https://github.com/docker/docker/compare/v20.10.17...v20.10.27">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/pachyderm/pachyderm/network/alerts). </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jose Molina-Melendez <jose.molina-melendez@hpe.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9966 +/- ##
==========================================
+ Coverage 55.05% 58.18% +3.13%
==========================================
Files 612 613 +1
Lines 74777 74892 +115
==========================================
+ Hits 41165 43578 +2413
+ Misses 33328 30757 -2571
- Partials 284 557 +273 ☔ View full report in Codecov by Sentry. |
This cleans up some logging. WithTx reports its retry attempt number as a field instead of being in the logger name, and associates with postgres pid and transaction id with any logs that are generated in the transaction. This is most useful with query logging enabled. WithTx is also a span now, so we can track the duration and success/failure status of individual transactions easily. The GRPC logging interceptor has been split into two parts. The first part runs early and creates a context containing the rpc name, request id, rpc peer address, etc. This means that any logs that other interceptors generate, notably auth, are associated with the RPC. The second part announces the start and end of the RPC, and runs after auth, so the authenticated username (if any) is always logged here. (The username is also logged for any other logs generated by the RPC handler.) This fixes an issue where database operations done as part of authorizing the RPC didn't show up with the request ID, etc. (Actually, that wouldn't have worked until #9947 anyway, so this is more of a fast follow on top of that.) It should now be possible to associate postgres logs and x-request-id, so we can have an end to end view of the system. Part of CORE-2248
…hyderm/pachyderm into acohen4/fix-dup-pip-versions
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.