Add a flag to vstream to exclude keyspace from table name

[twthorn]

Description

This adds a vstream flag to use the table name (without adding a keyspace) (and not copy/modify the vstream events received from the tablet). The goal is to optimize performance by skipping this copy step for clients that don't need it.

The benchmarked performance gain is substantial. This is because we do a deep copy of the entirety of every single row event. This may even be one of the main resource usages of vstream manager/vtgate process & could have cost savings as well.

Benchmarks

Benchmark results show 75% reduction in wall clock time to execute the same workload between exclude keyspace from table name enabled and the flag disabled disabled.

2268114250 / 9872382125 = 0.2297433609520053

$ go test -v -benchmem -run=^$ -bench ^BenchmarkVStreamEvents$ vitess.io/vitess/go/vt/vtgate
W0520 15:15:56.294720   19477 log.go:39] Failed to read in config : Config File "vtconfig" Not Found in "[/Users/tthornton/git/twthorn/vitess/go/vt/vtgate]". This is optional, and can be ignored if you are not using config files. For a detailed explanation, see https://github.com/vitessio/vitess/blob/main/doc/viper/viper.md#config-files.
goos: darwin
goarch: arm64
pkg: vitess.io/vitess/go/vt/vtgate
cpu: Apple M1 Max
BenchmarkVStreamEvents
BenchmarkVStreamEvents/ExcludeKeyspaceFromTableName=true
    vstream_manager_test.go:359: Received events 20000000, expected total 20000000
E0520 15:16:01.820125   19477 vstream_manager.go:413] Error in vstream for keyspace:"TestVStream" shard:"-20" gtid:"gtid-19900000": context canceled
context ended while streaming from TestVStream/-20
BenchmarkVStreamEvents/ExcludeKeyspaceFromTableName=true-10                    1        1589754084 ns/op        641058352 B/op  20004686 allocs/op
BenchmarkVStreamEvents/ExcludeKeyspaceFromTableName=false
    vstream_manager_test.go:359: Received events 20000000, expected total 20000000
E0520 15:16:12.012234   19477 vstream_manager.go:413] Error in vstream for keyspace:"TestVStream" shard:"-20" gtid:"gtid-19900000": context canceled
context ended while streaming from TestVStream/-20
BenchmarkVStreamEvents/ExcludeKeyspaceFromTableName=false-10                   1        6363939000 ns/op        7807126016 B/op 79678144 allocs/op
PASS
ok      vitess.io/vitess/go/vt/vtgate   16.935s

CPU shows 82% reduction
Using go pprof cpu profiler

Before: 1.91s (8.4%)
After: 350ms (2.6%)

Related Issue(s)

#18273

We could also, instead of wrapping in a flag, we could simply modify the original event received. This may have side effects. It also causes many unit tests to fail as they assume the passed in event is not changed. This would mean no extra flag/api changes and improve performance. Please let me know if we prefer this route and I'll put out another PR.

Note: we could add a validation check to prevent the ExcludeKeyspaceFromTableName flag from being set if there are multiple keyspaces being streamed. This could act as a safeguard. However, some clients may want the performance gain and are willing track client-side which table belongs to which keyspace. So the validation check would not be desirable in this case. Open to either option on this.

Checklist

I'll put out a doc PR once we agree on this and the naming of the flag etc.

• "Backport to:" labels have been added if this change should be back-ported to release branches
• If this change is to be back-ported to previous releases, a justification is included in the PR description
• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on CI?
• Documentation was added or is not required

Deployment Notes

[vitess-bot]

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

• Ensure that the Pull Request has a descriptive title.
• Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

• Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

• Apply the release notes (needs details) label if users need to know about this change.
• New features should be documented.
• There should be some code comments as to why things are implemented the way they are.
• There should be a comment at the top of each new or modified test to explain what the test does.

New flags

• Is this flag really necessary?
• Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

• Each item in Jobs should be named in order to mark it as required.
• If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• RPC changes should be compatible with vitess-operator
• If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
• vtctl command output order should be stable and awk-able.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.47%. Comparing base (fcfa0c8) to head (5addc42).
Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #18274      +/-   ##
==========================================
- Coverage   67.48%   67.47%   -0.01%     
==========================================
  Files        1603     1603              
  Lines      262337   262397      +60     
==========================================
+ Hits       177034   177053      +19     
- Misses      85303    85344      +41     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[rohit-nayak-ps]

Nice work!