[DNM] *: fix buildutil.VerifyNoImports, rework illegal imports #64450
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
|
This change is |
ajwerner
force-pushed
the
ajwerner/fix-dep_test
branch
from
d5f4234
to
6772ef9
Compare
The `geo` package depends on tons of stuff. The usage only needed geopb. Release note: None
The usage of `kvserver` in the `sql` package is generally forbidden. The tests to enforce that were broken. This patch reworks the dependency to move the business logic into the `kvserver` package. Release note: None
This setting was being imported from `sql` which created an illegal dependency. Release note: None
The dependency on these concepts was creating unintended dependencies on things like storage and pebble. Release note: None
ajwerner
force-pushed
the
ajwerner/fix-dep_test
branch
from
6772ef9
to
784d6b9
Compare
The linter was not happy about these. Release note: None
Release note: None
This thing has not worked since cockroachdb#49447 and the transition to go modules. This change adopts the newer `packages` API and enhances the abstraction a bit. Release note: None
Release note: None
ajwerner
force-pushed
the
ajwerner/fix-dep_test
branch
from
784d6b9
to
e000ce5
Compare
This was referenced Apr 30, 2021
craig bot
pushed a commit
that referenced
this pull request
Apr 30, 2021
63087: ui: use uPlot lib for line graphs on metrics page r=dhartunian a=dhartunian Previously, we were using the nvd3 library to render line graphs. This library is deprecated and relied on svg for rendering visuals. For clusters past a certain size (~30 nodes) performance on the metrics dashboards would become unusably slow due to the large amount of svg on the page. In addition, it looks like nvd3 would render a lot of unnecessary svg along with the lines to render points on hover which was difficult to circumvent. This PR replaces the `LineGraph` component with one that uses the uPlot library (https://github.com/leeoniya/uPlot). This library uses the canvas API to render and has been engineered with performance in mind for larger datasets. The implementation in this PR can render a 60-cluster Hardward metrics page quite quickly while retaining a guideline on hover with legend displaying the values at the guideline. Here's a list of important features implemented with uPlot in this commit - Plotting graphs with correctly formatted x-axis - Hovering legend on mouseover that mimics what we had before (legend redesign is pending future work) - Aggregation of multiple "result" time series from CRDB into a single set of y values aligned against a single time series (this is how uPlot requires its data to be formatted, we previously had separate time axes per plot series) - Interactive selection of zoom area on uPlot graph that triggers a zoom on the entire dashboard - uPlot uses same color palette as nvd3 implementation Removed features from nvd3 implementation: - Synced guideline on all graphs in dashboard Resolves #62463. Release note (ui change): use a new library for line graphs that renders metrics more efficiently. Customers with large clusters can now load and interact with metrics much faster than before. (TODO: what features are different with this new library?) 64391: clusterversion: introduce 21.2 development versions r=j-low a=j-low Release note: None 64466: kvserver,sql,server: rework dependencies for `compact_engine_span` r=ajwerner a=ajwerner The usage of `kvserver` in the `sql` package is generally forbidden. The tests to enforce that were broken. This patch reworks the dependency to move the business logic into the `kvserver` package. Part of #64450. Release note: None Co-authored-by: David Hartunian <davidh@cockroachlabs.com> Co-authored-by: Andrii Vorobiov <and.vorobiov@gmail.com> Co-authored-by: Joseph Lowinske <joe@cockroachlabs.com> Co-authored-by: Andrew Werner <awerner32@gmail.com>
This was referenced Feb 8, 2022
|
We've moved this into bazel. |
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.
The prefix of commits before fixing the testing infrastructure are attempts to break illegal imports
from sql and roachpb. I have a feeling there is more to do. I think the way I'd like to merge this is by fixing
the problems and then merging the fixed test. I'm headed out soon so I probably won't get to this next week
but it should be a good start.
The changes to fix the dependencies have been broken out into separate PRs:
compact_engine_span#64466SplitByLoadMergeDelayto kvserverbase #64467