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

ui: Add loading state to tables on database page #46568

Closed
1 task done
Annebirzin opened this issue Mar 25, 2020 · 0 comments · Fixed by #46857
Closed
1 task done

ui: Add loading state to tables on database page #46568

Annebirzin opened this issue Mar 25, 2020 · 0 comments · Fixed by #46857
Assignees
Labels
A-webui-general Issues on the DB Console that span multiple areas or don't have another clear category.

Comments

@Annebirzin
Copy link

Annebirzin commented Mar 25, 2020

  • Add loading state to individual tables of the database page (right now the message says this database has no tables while loading)

Figma designs

Databases_00

@Annebirzin Annebirzin added the A-webui-general Issues on the DB Console that span multiple areas or don't have another clear category. label Mar 25, 2020
@vladlos vladlos self-assigned this Apr 1, 2020
vladlos added a commit to vladlos/cockroach that referenced this issue Apr 2, 2020
Added global loading state to sort tables
Added loading state to database tables

Resolves: cockroachdb#46568

Release justification: low risk, high benefit changes to existing functionality

Release note (ui): database loading state design updates
craig bot pushed a commit that referenced this issue Apr 21, 2020
45865: backupccl: remove unused username param in WriteTableDescs r=dt a=pbardea

Release note: None

46857: ui: Sortable loading state r=dhartunian a=elkmaster

Added global loading state to sort tables
Added loading state to database tables

![loading-database](https://user-images.githubusercontent.com/12850886/78561401-4e875500-7820-11ea-8462-53bd7310adf0.png)

Resolves: #46568

Release justification: low risk, high benefit changes to existing functionality

Release note (ui): database loading state design updates

47582:  backupccl: fix flake in TestProtectedTimestampDuringBackup  r=pbardea a=pbardea

TestProtectedTimestampDuringBackup would sometimes flake as the GC queue
would assign it a low priority that would be below the threshold to turn
shouldQueue to true. However, the priority is non-zero indicating that
the timestamp was indeed not protected. This change aims to remove the
flake by checking for a non-zero priority rather than if shouldQueue is
true.

This PR also makes the same change to the ImportInto variant of the test since
they share the same test structure.

Fixes #47522.

Release note: None

47583: col*: use logical types throughout the vectorized engine r=yuzefovich a=yuzefovich

**col...: create new package and move some code**

This commit introduces `colbase` package which currently contains the
following (that are extracted from `colexec` package):
- `allocator.go`
- `random_testutils.go`
- `Operator` interface
- `CopyBatch` method.

The reason for extracting these things is that they are used by multiple
col* packages, so I think it's a good hygiene to separate them out.

This commit also renames `execerror` package to `vecerror` and then
moves it inside of `colbase` directory. It also changes the panic
matching so that now we catch all panics coming from a package that has
`pkg/sql/col` in its path (this will make sure that we don't forget to
register newly added packages that use panic-catching mechanism of the
vectorized engine with the panic-catcher). Additionally, this commit
renames the methods of `execerror` package.

Next, it moves `colexec/typeconv` package into `colbase` as well as
`colexec/testutils.go` file.

Finally, it removes `vecerror.NonVectorizedPanic` in favor of
`vecerror.ExpectedError`. The guidance on whether `InternalError` or
`ExpectedError` method should be used has been updated: the distinction
is whether the vectorized engine ends up in an unexpected - invalid -
state (for example, we expect that the vectorized engine might be
performing division by zero when evaluating a binary expression).

Release note: None

**col...: use logical types throughout the vectorized engine**

This commit transitions the vectorized engine to use SQL logical types
wherever possible, only converting to its physical type equivalent when
necessary (for example, when choosing which instance of projection
operator to use). This will allow us to have access to the actual type
whever we need and will allow us implement `coltypes.Datum` for all
unoptimized types.

This commit also moves some a few things around to clean up the
dependency graph (namely, `BytesEncodeFormat` is moved from
`sql/sessiondata` to `sql/lex`). Additionally, it replaces a single call
to `util/log.Infof` with a print statement in `util/protoutil` package
to remove the dependency on `util/log` (which depends on a bunch of
other things) so that `pkg/workload` would need to import less things.

Another thing worth calling out is the creation of copies of types in
`execplan` file to make sure that we don't override the input sync
specs.

Addresses: #43559.

Release note: None

**col...: introduce new package and more code movement**

Move `coldata/random_testutils.go` into newly created package
`coldatatestutils`.

Move contents of `colbase/random_testutils.go` into `coldatatestutils`
package.

Rename `colbase` to `colexecbase`. Also remove templated comments from
import sections of `_tmpl` files in favor of adding vars that remove
unused warnings (those templated comments work poorly when
moving/renaming the dependencies).

Rename `vecerror` to `colexecerror`.

Move `CopyBatch` from `colexecbase` into `coldatatestutils`. Also remove
memory accounting from `CopyBatch`.

Move `typeconv` from `colexecbase` into `coltypes` folder.

Move `coldata/vec_test.go` into `coldata_test` package to prevent an
import cycle. Also move one unit test from `coldata/vec_test.go` into
`coldata/bytes_test.go`.

Move `colexecbase/allocator.go` into newly created `colmem` package.

Release note: None

47629: sql: use Clock.PhysicalTime in beginTransactionTimestampsAndReadMode r=nvanbenschoten a=nvanbenschoten

Synchronizing with the HLC clock doesn't look to be necessary. I'm confused
about this though. The comment on beginTransactionTimestampsAndReadMode says
that "txnSQLTimestamp propagates to become the TxnTimestamp". Is this trying to
say that the timestamp makes it way into the kv.Txn? Because that's not true.

Regardless, the one reason not to make this change is that PhysicalTime is not
guaranteed to be monotonic on some systems and can generally diverge from the
HLC's clock. If we're worried about that though, we should use the HLC here and
feed that directly into the kv.Txn. We shouldn't need to grab two timestamps
from the HLC per txn.

47756: cli,base: surface stored critical errors at the right moment r=tbg a=knz

Fixes #44041

If/when storage detects an important error, the text for this error is
stored in a file named `_CRITICAL_ERROR.txt` in the auxiliary
directory. The intention is to block subsequent server restarts until
the error is investigated and the file (manually) removed.

Prior to this patch, this check was done in the startup sequence 1)
before logging was fully initialized 2) using a `log.Fatal` to
announce the critical error.

The first aspect is problematic because it logs before logging flags
are applied. The second is problematic because it makes the failure
super-verbose and buries the lede.

This patch simplifies the code and makes the error reported at the
right place.

Example, before:
```
kena@kenax ....com/cockroachdb/cockroach % ./cockroach start-single-node
F200421 14:24:02.303675 1 cli/start.go:478  From .../auxiliary/_CRITICAL_ALERT.txt:

boom

goroutine 1 [running]:
github.com/cockroachdb/cockroach/pkg/util/log.getStacks(0x6a7ca00, 0xed630f902, 0x0, 0x1000)
        ...//src/github.com/cockroachdb/cockroach/pkg/util/log/get_stacks.go:25 +0xb8
github.com/cockroachdb/cockroach/pkg/util/log.(*loggerT).outputLogEntry(0x6a79800, 0xc000000004, 0x33eb7b9, 0xc, 0x1de, 0xc000ba8180, 0x76)
        ...//src/github.com/cockroachdb/cockroach/pkg/util/log/clog.go:210 +0xa92
github.com/cockroachdb/cockroach/pkg/util/log.addStructured(0x15e3420, 0xc000078168, 0x4, 0x2, 0x0, 0x0, 0xc00063f7e8, 0x1, 0x1)
        ...//src/github.com/cockroachdb/cockroach/pkg/util/log/structured.go:66 +0x2c9
...
[147/245]
****************************************************************************

This node experienced a fatal error (printed above), and as a result the
process is terminating.

Fatal errors can occur due to faulty hardware (disks, memory, clocks) or a
...
    support@cockroachlabs.com

The Cockroach Labs team appreciates your feedback.
```

Example, after:

```
kena@kenax ....com/cockroachdb/cockroach % ./cockroach start-single-node
*
* ERROR: startup forbidden by prior critical alert
* DETAIL: From /data/home/kena/src/go/src/github.com/cockroachdb/cockroach/cockroach-data/auxiliary/_CRITICAL_ALERT.txt:
* boom
*
```

Release note (cli change): The error message displayed upon `cockroach
start` / `cockroach start-single-node` when manual intervention is
needed in the store directory is now clearer.

Co-authored-by: Paul Bardea <pbardea@gmail.com>
Co-authored-by: Vlad Los <carrott9@gmail.com>
Co-authored-by: Paul Bardea <paul@pbardea.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
@craig craig bot closed this as completed in 97212f2 Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-webui-general Issues on the DB Console that span multiple areas or don't have another clear category.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants