From f8bb122de2df80979154fcd451711edaeca73ac5 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Thu, 14 Nov 2024 18:19:23 -0600 Subject: [PATCH] Do not run pr_base benchmarks and compare with most recent cron Signed-off-by: Florent Poinsard --- go/exec/exec.go | 60 ++++++++++++++++++++++++++++++++----- go/exec/pull_request.go | 44 +++++++++++++++++++++++---- go/server/api.go | 9 +++--- go/server/cron_execution.go | 1 - go/server/cron_handlers.go | 20 +++---------- go/tools/github/github.go | 27 ++++++++++------- 6 files changed, 117 insertions(+), 44 deletions(-) diff --git a/go/exec/exec.go b/go/exec/exec.go index fa136d6ed..3adfe4fe7 100644 --- a/go/exec/exec.go +++ b/go/exec/exec.go @@ -74,8 +74,7 @@ type Exec struct { Workload string // PullNB defines the pull request number linked to this execution. - PullNB int - PullBaseBranchRef string + PullNB int // Configuration used to interact with the SQL database. configDB *psdb.Config @@ -162,11 +161,10 @@ type ProfileInformation struct { const ( MaximumBenchmarkWithSameConfig = 10 - SourceCron = "cron" - SourcePullRequest = "cron_pr" - SourcePullRequestBase = "cron_pr_base" - SourceTag = "cron_tags_" - SourceReleaseBranch = "cron_" + SourceCron = "cron" + SourcePullRequest = "cron_pr" + SourceTag = "cron_tags_" + SourceReleaseBranch = "cron_" ) // NewExec creates a new *Exec given the string representation of an uuid.UUID. @@ -543,6 +541,54 @@ func GetPreviousFromSourceMacrobenchmark(client storage.SQLClient, source, workl return } +func getGitRefOfLatestFinishedMatchingSource(client storage.SQLClient, source string) (gitRefOut string, err error) { + query := "SELECT e.git_ref FROM execution e WHERE e.source = ? AND e.status = 'finished' ORDER BY e.started_at DESC LIMIT 1" + result, err := client.Read(query, source) + if err != nil { + return + } + defer result.Close() + for result.Next() { + err = result.Scan(&gitRefOut) + if err != nil { + return + } + } + return +} + +func getGitRefOfFinishedMatchingSourceGivenTimestamp(client storage.SQLClient, source string, t *time.Time) (gitRefOut string, err error) { + query := ` + SELECT e.git_ref + FROM execution e + WHERE e.source = ? + AND e.status = 'finished' + AND e.started_at < ? + AND e.started_at = ( + SELECT MIN(sub.started_at) + FROM execution sub + WHERE sub.git_ref = e.git_ref + AND sub.source = e.source + AND sub.status = e.status + AND sub.started_at < ? + ) + ORDER BY e.started_at DESC + LIMIT 1 + ` + result, err := client.Read(query, source, t.Format(time.DateTime), t.Format(time.DateTime)) + if err != nil { + return + } + defer result.Close() + for result.Next() { + err = result.Scan(&gitRefOut) + if err != nil { + return + } + } + return +} + // GetLatestDailyJobForMacrobenchmarks will fetch and return the commit sha for which // the last daily job for macrobenchmarks was run func GetLatestDailyJobForMacrobenchmarks(client storage.SQLClient) (gitSha string, err error) { diff --git a/go/exec/pull_request.go b/go/exec/pull_request.go index 5a848e83c..9e9bdefd3 100644 --- a/go/exec/pull_request.go +++ b/go/exec/pull_request.go @@ -18,7 +18,13 @@ package exec -import "github.com/vitessio/arewefastyet/go/storage" +import ( + "errors" + "regexp" + + "github.com/vitessio/arewefastyet/go/storage" + "github.com/vitessio/arewefastyet/go/tools/github" +) func GetPullRequestList(client storage.SQLClient) ([]int, error) { rows, err := client.Read("select pull_nb from execution where pull_nb > 0 group by pull_nb order by pull_nb desc") @@ -41,12 +47,24 @@ func GetPullRequestList(client storage.SQLClient) ([]int, error) { } type pullRequestInfo struct { - Main string - PR string + Base string + Head string } -func GetPullRequestInfo(client storage.SQLClient, pullNumber int) (pullRequestInfo, error) { - rows, err := client.Read("select cron_pr.git_ref as pr, cron_pr_base.git_ref as main from (select git_ref from execution where pull_nb = ? and status = 'finished' and source = 'cron_pr' order by started_at desc limit 1) cron_pr , (select git_ref from execution where pull_nb = ? and status = 'finished' and source = 'cron_pr_base' order by started_at desc limit 1) cron_pr_base ", pullNumber, pullNumber) +func getSourceFromBranchName(branch string) (string, error) { + if branch == "main" { + return SourceCron, nil + } + matchRelease := regexp.MustCompile(`(release-[0-9]+.0)`) + matches := matchRelease.FindStringSubmatch(branch) + if len(matches) == 2 { + return SourceReleaseBranch + matches[1] + "-branch", nil + } + return "", errors.New("no match found") +} + +func GetPullRequestInfo(client storage.SQLClient, pullNumber int, info github.PRInfo) (pullRequestInfo, error) { + rows, err := client.Read("select git_ref from execution where pull_nb = ? and status = 'finished' and source = 'cron_pr' order by started_at desc limit 1", pullNumber) if err != nil { return pullRequestInfo{}, err } @@ -55,10 +73,24 @@ func GetPullRequestInfo(client storage.SQLClient, pullNumber int) (pullRequestIn var res pullRequestInfo if rows.Next() { - err = rows.Scan(&res.PR, &res.Main) + err = rows.Scan(&res.Head) if err != nil { return pullRequestInfo{}, err } } + + source, err := getSourceFromBranchName(info.BaseRef) + if err != nil { + return res, nil + } + + if info.IsMerged { + res.Base, err = getGitRefOfFinishedMatchingSourceGivenTimestamp(client, source, info.MergedTime) + } else { + res.Base, err = getGitRefOfLatestFinishedMatchingSource(client, source) + } + if err != nil { + return pullRequestInfo{}, err + } return res, nil } diff --git a/go/server/api.go b/go/server/api.go index 608babc48..312e2194a 100644 --- a/go/server/api.go +++ b/go/server/api.go @@ -348,21 +348,22 @@ func (s *Server) getPullRequestInfo(c *gin.Context) { slog.Error(err) return } - gitPRInfo, err := exec.GetPullRequestInfo(s.dbClient, pullNb) + prInfo, err := s.ghApp.GetPullRequestInfo(pullNb) if err != nil { c.JSON(http.StatusInternalServerError, &ErrorAPI{Error: err.Error()}) slog.Error(err) return } - prInfo, err := s.ghApp.GetPullRequestInfo(pullNb) + gitPRInfo, err := exec.GetPullRequestInfo(s.dbClient, pullNb, prInfo) if err != nil { c.JSON(http.StatusInternalServerError, &ErrorAPI{Error: err.Error()}) slog.Error(err) return } - prInfo.Base = gitPRInfo.Main - prInfo.Head = gitPRInfo.PR + + prInfo.Base = gitPRInfo.Base + prInfo.Head = gitPRInfo.Head c.JSON(http.StatusOK, prInfo) } diff --git a/go/server/cron_execution.go b/go/server/cron_execution.go index c3096a87a..7089be975 100644 --- a/go/server/cron_execution.go +++ b/go/server/cron_execution.go @@ -52,7 +52,6 @@ func (s *Server) executeSingle(config benchmarkConfig, identifier executionIdent e.GitRef = identifier.GitRef e.VtgatePlannerVersion = identifier.PlannerVersion e.PullNB = identifier.PullNb - e.PullBaseBranchRef = identifier.PullBaseRef e.VitessVersion = identifier.Version e.NextBenchmarkIsTheSame = nextIsSame e.ProfileInformation = identifier.Profile diff --git a/go/server/cron_handlers.go b/go/server/cron_handlers.go index 7df654359..4a0e443cb 100644 --- a/go/server/cron_handlers.go +++ b/go/server/cron_handlers.go @@ -230,14 +230,14 @@ func (s *Server) pullRequestsCronHandler() { } if workload == "micro" { - elements = append(elements, s.createPullRequestElementWithBaseComparison(config, ref, workload, previousGitRef, "", pullNb, currVersion)...) + elements = append(elements, s.createPullRequestElement(config, ref, workload, "", pullNb, currVersion)) } else { versions := []macrobench.PlannerVersion{macrobench.V3Planner} if labelInfo.useGen4 { versions = []macrobench.PlannerVersion{macrobench.Gen4Planner} } for _, version := range versions { - elements = append(elements, s.createPullRequestElementWithBaseComparison(config, ref, workload, previousGitRef, version, pullNb, currVersion)...) + elements = append(elements, s.createPullRequestElement(config, ref, workload, version, pullNb, currVersion)) } } } @@ -249,20 +249,8 @@ func (s *Server) pullRequestsCronHandler() { } } -func (s *Server) createPullRequestElementWithBaseComparison(config benchmarkConfig, ref, workload, previousGitRef string, plannerVersion macrobench.PlannerVersion, pullNb int, gitVersion git.Version) []*executionQueueElement { - var elements []*executionQueueElement - - newExecutionElement := s.createSimpleExecutionQueueElement(config, exec.SourcePullRequest, ref, workload, string(plannerVersion), true, pullNb, gitVersion, nil) - newExecutionElement.identifier.PullBaseRef = previousGitRef - elements = append(elements, newExecutionElement) - - if previousGitRef != "" { - previousElement := s.createSimpleExecutionQueueElement(config, exec.SourcePullRequestBase, previousGitRef, workload, string(plannerVersion), false, pullNb, gitVersion, nil) - previousElement.compareWith = append(previousElement.compareWith, newExecutionElement.identifier) - newExecutionElement.compareWith = append(newExecutionElement.compareWith, previousElement.identifier) - elements = append(elements, previousElement) - } - return elements +func (s *Server) createPullRequestElement(config benchmarkConfig, ref, workload string, plannerVersion macrobench.PlannerVersion, pullNb int, gitVersion git.Version) *executionQueueElement { + return s.createSimpleExecutionQueueElement(config, exec.SourcePullRequest, ref, workload, string(plannerVersion), true, pullNb, gitVersion, nil) } func (s *Server) tagsCronHandler() { diff --git a/go/tools/github/github.go b/go/tools/github/github.go index f3cae6081..c8fe6c239 100644 --- a/go/tools/github/github.go +++ b/go/tools/github/github.go @@ -113,12 +113,15 @@ func (a *App) Init() error { } type PRInfo struct { - ID int - Author string - Title string - CreatedAt *time.Time - Base string - Head string + ID int + Author string + Title string + CreatedAt *time.Time + Base string + BaseRef string + Head string + IsMerged bool + MergedTime *time.Time } func (a *App) GetPullRequestInfo(prNumber int) (PRInfo, error) { @@ -129,10 +132,14 @@ func (a *App) GetPullRequestInfo(prNumber int) (PRInfo, error) { } createAt := pr.GetCreatedAt().Time + mergedAt := pr.GetMergedAt() return PRInfo{ - ID: prNumber, - Author: pr.User.GetLogin(), - Title: pr.GetTitle(), - CreatedAt: &createAt, + ID: prNumber, + Author: pr.User.GetLogin(), + Title: pr.GetTitle(), + CreatedAt: &createAt, + IsMerged: pr.GetMerged(), + MergedTime: mergedAt.GetTime(), + BaseRef: pr.GetBase().GetRef(), }, nil }