Skip to content

[Do not merge] v19.x #127

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

Open
wants to merge 37 commits into
base: release-19.0
Choose a base branch
from
Open

Conversation

arthurschreiber
Copy link
Member

Description

This pull request is to help us track backported changes.

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

Copy link

This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:

  • Push additional commits to the associated branch.
  • Remove the stale label.
  • Add a comment indicating why it is not stale.

If no action is taken within 7 days, this PR will be closed.

@github-actions github-actions bot added the Stale label Dec 20, 2024
Copy link

This PR was closed because it has been stale for 7 days with no activity.

@github-actions github-actions bot closed this Dec 27, 2024
@github-actions github-actions bot removed the Stale label Jan 24, 2025
mhamza15 and others added 14 commits February 3, 2025 15:08
Signed-off-by: Arthur Schreiber <[email protected]>
Signed-off-by: Arthur Schreiber <[email protected]>
Signed-off-by: Arthur Schreiber <[email protected]>
Use ubuntu 22.04 for all action workflows on v19.
Pull in latest upstream v19 changes
[release-19.0] Implement temporal comparisons (vitessio#17826) (vitessio#17852)
…zed strings in row events for unit tests (vitessio#14903)

Signed-off-by: Rohit Nayak <[email protected]>
…tctldclient (vitessio#16920) (vitessio#17015)

Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com>
…ableACLConfig is true (vitessio#17274)

Signed-off-by: garfthoffman <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Co-authored-by: Mohamed Hamza <[email protected]>
arthurschreiber and others added 6 commits April 7, 2025 18:59
Fail loading an ACL config if the provided file is empty and enforceT…
…itessio#18073) (#155)

Signed-off-by: Arthur Schreiber <[email protected]>
Signed-off-by: Mohamed Hamza <[email protected]>
Co-authored-by: Arthur Schreiber <[email protected]>
…thub

[release-19.0-github] Improve `UNION` query merging
Copy link

This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:

  • Push additional commits to the associated branch.
  • Remove the stale label.
  • Add a comment indicating why it is not stale.

If no action is taken within 7 days, this PR will be closed.

@github-actions github-actions bot added the Stale label Jun 27, 2025
Copy link

github-actions bot commented Jul 5, 2025

This PR was closed because it has been stale for 7 days with no activity.

@github-actions github-actions bot closed this Jul 5, 2025
sqlparser: Remove unneeded escaping (vitessio#16255)
@github-actions github-actions bot removed the Stale label Jul 24, 2025
@Copilot Copilot AI review requested due to automatic review settings August 1, 2025 12:12
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This is a backport pull request for tracking changes that need to be applied to the v19.x branch of Vitess. The PR includes various fixes, improvements, and test updates across multiple components including vstreamer, throttling, logging, CI/CD configuration, and test frameworks.

  • Updates CI templates to use Ubuntu 22.04 instead of 24.04 and adds repository owner checks for self-hosted runners
  • Removes deprecated log.Flush() calls and unused timer functionality from vstreamer and vreplication
  • Introduces new test framework for vstreamer event validation and improves test coverage

Reviewed Changes

Copilot reviewed 187 out of 188 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tools/rowlog/rowlog.go Removes deprecated log.Flush() call
test/templates/*.tpl Updates CI runner configurations and adds repository ownership checks
go/vt/wrangler/*_test.go Fixes regex escaping in test expectations
go/vt/vttablet/tabletserver/vstreamer/* Major refactoring with new test framework and throttling improvements
go/vt/vttablet/tabletserver/throttle/* Enhanced throttling error detection and test coverage
go/vt/vttablet/tabletserver/tabletenv/* Updated logging format to use structured JSON output
go/vt/vttablet/tabletmanager/vreplication/* Improved throttling metrics and lag calculation
go/vt/vtgate/* Enhanced gateway functionality and buffering logic

@@ -26,6 +26,8 @@ import (
"testing"
"time"

"github.com/prometheus/common/version"

Copy link
Preview

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

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

The import of 'github.com/prometheus/common/version' appears to be unused in this file. Consider removing it unless it's used elsewhere in the code that wasn't shown in the diff.

Suggested change

Copilot uses AI. Check for mistakes.

@@ -1721,6 +1587,9 @@ func TestBestEffortNameInFieldEvent(t *testing.T) {

// test that vstreamer ignores tables created by OnlineDDL
func TestInternalTables(t *testing.T) {
if version.GoOS == "darwin" {
Copy link
Preview

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

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

Using 'version.GoOS' from prometheus package instead of the standard 'runtime.GOOS' is unconventional. Consider using 'runtime.GOOS' for OS detection as it's the standard Go approach.

Suggested change
if version.GoOS == "darwin" {
if runtime.GOOS == "darwin" {

Copilot uses AI. Check for mistakes.


// Run() runs the test. It first initializes the test, then runs the queries and validates the events.
func (ts *TestSpec) Run() {
require.NoError(ts.t, engine.se.Reload(context.Background()))
Copy link
Preview

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

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

The test framework assumes global variables 'engine' exist. Consider passing these dependencies explicitly to improve testability and reduce coupling to global state.

Copilot uses AI. Check for mistakes.

// Close() should be called (via defer) at the end of the test to clean up the tables created in the test.
func (ts *TestSpec) Close() {
dropStatement := fmt.Sprintf("drop tables %s", strings.Join(ts.schema.TableNames(), ", "))
execStatement(ts.t, dropStatement)
Copy link
Preview

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

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

[nitpick] The 'execStatement' function is called but not defined in this file. Consider adding error handling or documentation about where this function is defined to improve code clarity.

Copilot uses AI. Check for mistakes.

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.