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

release-21.1: sql: various event log fixes #65554

Merged
merged 11 commits into from
May 21, 2021

Conversation

knz
Copy link
Contributor

@knz knz commented May 21, 2021

Backport:

Please see individual PRs for details.

/cc @cockroachdb/release

fqazi and others added 11 commits May 21, 2021 11:22
Fixes: cockroachdb#58496

Previously, the object name information for TableIndexName,
was not populated in multiple contexts leading to incorrect
formatting of index names in event log messages. To address
this, this patch modifies code paths for resolving table
indexes to add this information during the planning phase.

Release note: None
Helps with cockroachdb#41930.

Previously, if we ran grant/revoke on multiple tables,
we would create event logs for each table and write them
one by one, resulting in round trips proportional to
the number of tables.
This patch addresses this by batch writing the event logs,
so that 1 write to the event log table occurs regardless
of the number of tables updated.

Release note: None
Release note (sql change): The statement type ("tag") is now also
included alongside the full text of the SQL query in the various
structured log entries produced when query execution is being logged.
Prior to this patch, there was a little mess in `sql/event_log.go`
that had been introduced when "optimizing" GRANT/REVOKE to only
use one write batch for all the events logged:

- the API interface for the functions in event_log.go
  were complex to use, requiring callers to provide descriptor
  IDs and structured events as separate slices;
- the optimizing logic was not properly applied to the
  other case where multiple events are emitted:
  SQL audit logging in `exec_log.go`.

This patch cleans this up by using the same struct `eventLogEntry` as
argument to the various APIs.

Release note: None
…ging

The previous patch to batch event writes for GRANT/REVOKE had
duplicated code. This was not necessary. This patch fixes this by
using the same code for both cases.

Release note: None
This clarifies its purpose.

Release note: None
`logEventsOnlyExternally()` is specific to the code in `exec_log.go`
and should thus reside there. This patch achieves that.

Release note: None
The code was previously using two booleans `onlyLog` and
`writeToEventLog` which were making the code difficult to understand
and to maintain.

This patch fixes this by introducing a bitset with descriptive names.

Release note: None
This further simplifies the internal API.

Release note: None
This patch describes `event_log.go` at a high level: an overall event
refinement pipeline with a straightforward control flow.

Release note: None
This patch fixes a bug introduced when query logging
started using structured events: the ability to automatically
copy all the execution events to the DEV channel when
setting the `vmodule` setting to `exec_log=2` (or above).

In addition to fixing that bug, the following new vmodule-based
abilities are added:

- events for DDL statements and others that call `logEvent()` can now
  be collected in the DEV channel by using the name of the source file
  where they were generated as filter (e.g. `vmodule=create_table=2` for
  the CREATE TABLE events.
- events of other kinds can be collected in the DEV channel
  by setting `vmodule=event_log=2`.

(Note a subtle difference between `vmodule=create_table=2` and
`vmodule=exec_log=2`: the former emits the event to the DEV channel
while the stmt is executed; the latter emits the event after the stmt
completes. If both are enabled, TWO events are sent to the DEV channel.)

Since all the vmodule filtering options are subject to change without
notice between versions, we do not wish to document these nuances.
For this reason, the release note below is left blank.

Release note: None
@knz knz requested a review from rafiss May 21, 2021 09:24
@knz knz requested a review from a team as a code owner May 21, 2021 09:24
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@knz knz merged commit 9a6c8c0 into cockroachdb:release-21.1 May 21, 2021
@knz knz deleted the backport21.1-61776-63743-64871-64894 branch May 21, 2021 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants