From 02921de947f5f2f75781ae99bcefca04b88ca72d Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 28 Oct 2024 17:06:22 -0600 Subject: [PATCH 01/12] Add a route to clear the execution queue on demand Signed-off-by: Florent Poinsard --- docker-compose.yml | 1 + go/admin/admin.go | 11 +- go/admin/api.go | 111 ++++++++++++++++-- .../{content.html => add_new_executions.html} | 0 go/admin/templates/base.html | 6 +- go/admin/templates/clear_queue.html | 31 +++++ go/admin/templates/sidebar.html | 11 +- go/server/api.go | 48 ++++++-- go/server/cron.go | 11 ++ go/server/server.go | 1 + 10 files changed, 199 insertions(+), 32 deletions(-) rename go/admin/templates/{content.html => add_new_executions.html} (100%) create mode 100644 go/admin/templates/clear_queue.html diff --git a/docker-compose.yml b/docker-compose.yml index 932f39f4..0f316600 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -64,6 +64,7 @@ services: volumes: - "./config/dev/config.yaml:/config/config.yaml" - "./config/dev/secrets.yaml:/config/secrets.yaml" + - "./go/admin/templates/:/go/admin/templates/" labels: - "traefik.enable=true" - "traefik.http.routers.admin.rule=Host(`localhost`)" diff --git a/go/admin/admin.go b/go/admin/admin.go index be397db5..8a652178 100644 --- a/go/admin/admin.go +++ b/go/admin/admin.go @@ -123,12 +123,19 @@ func (a *Admin) Run() error { MaxAge: 12 * time.Hour, })) - // API + // OAuth a.router.GET("/admin", a.login) a.router.GET("/admin/login", a.handleGitHubLogin) a.router.GET("/admin/auth/callback", a.handleGitHubCallback) + + // API a.router.POST("/admin/executions/add", a.authMiddleware(), a.handleExecutionsAdd) - a.router.GET("/admin/dashboard", a.authMiddleware(), a.dashboard) + a.router.POST("/admin/executions/clear", a.authMiddleware(), a.handleClearQueue) + + // Pages + a.router.GET("/admin/dashboard", a.authMiddleware(), a.homePage) + a.router.GET("/admin/dashboard/newexec", a.authMiddleware(), a.newExecutionsPage) + a.router.GET("/admin/dashboard/clearqueue", a.authMiddleware(), a.clearQueuePage) return a.router.Run(":" + a.port) } diff --git a/go/admin/api.go b/go/admin/api.go index 6a4a94a0..a8701acf 100644 --- a/go/admin/api.go +++ b/go/admin/api.go @@ -53,22 +53,36 @@ const ( arewefastyetTeamGitHub = "arewefastyet" ) -type ExecutionRequest struct { - Auth string `json:"auth"` - Source string `json:"source"` - SHA string `json:"sha"` - Workloads []string `json:"workloads"` - NumberOfExecutions string `json:"number_of_executions"` -} +type ( + executionRequest struct { + Auth string `json:"auth"` + Source string `json:"source"` + SHA string `json:"sha"` + Workloads []string `json:"workloads"` + NumberOfExecutions string `json:"number_of_executions"` + } + + clearQueueRequest struct { + Auth string `json:"auth"` + } +) func (a *Admin) login(c *gin.Context) { a.render(c, gin.H{}, "login.html") } -func (a *Admin) dashboard(c *gin.Context) { +func (a *Admin) homePage(c *gin.Context) { a.render(c, gin.H{}, "base.html") } +func (a *Admin) newExecutionsPage(c *gin.Context) { + a.render(c, gin.H{"Page": "newexec"}, "base.html") +} + +func (a *Admin) clearQueuePage(c *gin.Context) { + a.render(c, gin.H{"Page": "clearqueue"}, "base.html") +} + func CreateGhClient(token *oauth2.Token) *goGithub.Client { return goGithub.NewClient(oauthConf.Client(context.Background(), token)) } @@ -242,7 +256,7 @@ func (a *Admin) handleExecutionsAdd(c *gin.Context) { return } - requestPayload := ExecutionRequest{ + requestPayload := executionRequest{ Auth: encryptedToken, Source: source, SHA: sha, @@ -258,10 +272,7 @@ func (a *Admin) handleExecutionsAdd(c *gin.Context) { return } - serverAPIURL := "http://traefik/api/executions/add" - if a.Mode == server.ProductionMode { - serverAPIURL = "https://benchmark.vitess.io/api/executions/add" - } + serverAPIURL := getAPIURL(a.Mode, "/executions/add") req, err := http.NewRequest("POST", serverAPIURL, bytes.NewBuffer(jsonData)) if err != nil { @@ -289,3 +300,77 @@ func (a *Admin) handleExecutionsAdd(c *gin.Context) { c.JSON(http.StatusCreated, gin.H{"message": "Execution(s) added successfully"}) } + +func (a *Admin) handleClearQueue(c *gin.Context) { + tokenKey, err := c.Cookie("tk") + if err != nil { + slog.Error("Failed to get token from cookie: ", err) + c.AbortWithStatus(http.StatusUnauthorized) + return + } + + mu.Lock() + token, exists := tokens[tokenKey] + mu.Unlock() + + if !exists { + slog.Error("Failed to get token from map") + c.AbortWithStatus(http.StatusUnauthorized) + return + } + + encryptedToken, err := server.Encrypt(token.AccessToken, a.auth) + if err != nil { + slog.Error("Failed to encrypt token: ", err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to encrypt token"}) + return + } + + requestPayload := clearQueueRequest{ + Auth: encryptedToken, + } + + jsonData, err := json.Marshal(requestPayload) + + if err != nil { + slog.Error("Failed to marshal request payload: ", err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to marshal request payload"}) + return + } + + serverAPIURL := getAPIURL(a.Mode, "/executions/clear") + + req, err := http.NewRequest("POST", serverAPIURL, bytes.NewBuffer(jsonData)) + if err != nil { + slog.Error("Failed to create new HTTP request: ", err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create request to server API"}) + return + } + req.Header.Set("Content-Type", "application/json") + + client := &http.Client{} + resp, err := client.Do(req) + if err != nil { + slog.Error("Failed to send request to server API: ", err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to send request to server API"}) + return + } + + defer resp.Body.Close() + + if resp.StatusCode != http.StatusAccepted { + slog.Error("Server API returned an error: ", resp.Status) + c.JSON(resp.StatusCode, gin.H{"error": "Failed to process request on server API"}) + return + } + + c.JSON(http.StatusAccepted, gin.H{"message": "Execution(s) added successfully"}) +} + +func getAPIURL(mode server.Mode, endpoint string) string { + serverAPIURL := "http://traefik/api" + endpoint + if mode == server.ProductionMode { + serverAPIURL = "https://benchmark.vitess.io/api" + endpoint + } + return serverAPIURL +} diff --git a/go/admin/templates/content.html b/go/admin/templates/add_new_executions.html similarity index 100% rename from go/admin/templates/content.html rename to go/admin/templates/add_new_executions.html diff --git a/go/admin/templates/base.html b/go/admin/templates/base.html index 146031e9..df73b7d6 100644 --- a/go/admin/templates/base.html +++ b/go/admin/templates/base.html @@ -16,7 +16,11 @@ {{ template "header.html" . }}
{{ template "sidebar.html" . }}
-
{{ template "content.html" . }}
+ {{ if eq .Page "newexec" }} +
{{ template "add_new_executions.html" . }}
+ {{ else if eq .Page "clearqueue" }} +
{{ template "clear_queue.html" . }}
+ {{ end }}
diff --git a/go/admin/templates/clear_queue.html b/go/admin/templates/clear_queue.html new file mode 100644 index 00000000..a322ba8c --- /dev/null +++ b/go/admin/templates/clear_queue.html @@ -0,0 +1,31 @@ +
+
+ +
+
+
+ + \ No newline at end of file diff --git a/go/admin/templates/sidebar.html b/go/admin/templates/sidebar.html index 684a557a..3a36803f 100644 --- a/go/admin/templates/sidebar.html +++ b/go/admin/templates/sidebar.html @@ -1,9 +1,10 @@
diff --git a/go/server/api.go b/go/server/api.go index 76b9c4f2..cf1cf3b4 100644 --- a/go/server/api.go +++ b/go/server/api.go @@ -19,6 +19,7 @@ package server import ( + "errors" "fmt" "net/http" "sort" @@ -555,16 +556,8 @@ func (s *Server) addExecutions(c *gin.Context) { return } - decryptedToken, err := server.Decrypt(req.Auth, s.ghTokenSalt) - if err != nil { - slog.Error(err) - c.JSON(http.StatusUnauthorized, &ErrorAPI{Error: "Unauthorized"}) - return - } - - isUserAuthenticated, err := IsUserAuthenticated(decryptedToken) - if err != nil || !isUserAuthenticated { - c.JSON(http.StatusUnauthorized, &ErrorAPI{Error: "isUserAuthenticated Unauthorized"}) + if err := s.handleAuthentication(c, req.Auth); err != nil { + c.JSON(http.StatusUnauthorized, &ErrorAPI{Error: err.Error()}) return } @@ -596,8 +589,41 @@ func (s *Server) addExecutions(c *gin.Context) { c.JSON(http.StatusCreated, "") } -func IsUserAuthenticated(accessToken string) (bool, error) { +func (s *Server) clearExecutionQueue(c *gin.Context) { + var req struct { + Auth string `json:"auth"` + } + + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid request body"}) + return + } + if err := s.handleAuthentication(c, req.Auth); err != nil { + c.JSON(http.StatusUnauthorized, &ErrorAPI{Error: err.Error()}) + return + } + + s.clearQueue() + + c.JSON(http.StatusAccepted, "") +} + +func (s *Server) handleAuthentication(c *gin.Context, auth string) error { + decryptedToken, err := server.Decrypt(auth, s.ghTokenSalt) + if err != nil { + c.JSON(http.StatusUnauthorized, &ErrorAPI{Error: "Unauthorized"}) + return errors.New("unauthenticated") + } + + isUserAuthenticated, err := IsUserAuthenticated(decryptedToken) + if err != nil || !isUserAuthenticated { + return errors.New("unauthenticated") + } + return nil +} + +func IsUserAuthenticated(accessToken string) (bool, error) { client := &http.Client{} req, err := http.NewRequest("GET", "https://api.github.com/user", nil) if err != nil { diff --git a/go/server/cron.go b/go/server/cron.go index 48ce9be8..9c918eaa 100644 --- a/go/server/cron.go +++ b/go/server/cron.go @@ -191,3 +191,14 @@ func (s *Server) appendToQueue(execElements []*executionQueueElement) { time.Sleep(100 * time.Millisecond) } } + +func (s *Server) clearQueue() { + mtx.Lock() + defer mtx.Unlock() + for id, e := range queue { + if e.Executing { + continue + } + delete(queue, id) + } +} diff --git a/go/server/server.go b/go/server/server.go index ddd5fb6e..99df4ff3 100644 --- a/go/server/server.go +++ b/go/server/server.go @@ -254,6 +254,7 @@ func (s *Server) Run() error { s.router.GET("/api/run/request", s.requestRun) s.router.GET("/api/run/delete", s.deleteRun) s.router.POST("/api/executions/add", s.addExecutions) + s.router.POST("/api/executions/clear", s.clearExecutionQueue) return s.router.Run(":" + s.port) } From 889683242441b540bc588f6b497082ef62eabfea Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 28 Oct 2024 17:52:11 -0600 Subject: [PATCH 02/12] Enable pprof on demand Signed-off-by: Florent Poinsard --- ansible/roles/vtgate/templates/vtgate.conf.j2 | 3 + .../roles/vttablet/templates/vttablet.conf.j2 | 8 +-- go/admin/api.go | 35 +++++----- go/admin/templates/add_new_executions.html | 66 +++++++++++++++---- go/exec/exec.go | 12 ++++ go/infra/ansible/vars.go | 6 ++ go/server/api.go | 15 ++++- go/server/cron.go | 1 + go/server/cron_execution.go | 1 + go/server/cron_handlers.go | 17 ++--- 10 files changed, 123 insertions(+), 41 deletions(-) diff --git a/ansible/roles/vtgate/templates/vtgate.conf.j2 b/ansible/roles/vtgate/templates/vtgate.conf.j2 index ab99793f..5a4e49e3 100644 --- a/ansible/roles/vtgate/templates/vtgate.conf.j2 +++ b/ansible/roles/vtgate/templates/vtgate.conf.j2 @@ -22,6 +22,9 @@ EXTRA_VTGATE_FLAGS="\ --vschema_ddl_authorized_users \"%\" \ --vtgate-config-terse-errors \ --tablet_types_to_wait REPLICA \ + {% if vitess_profile_binary == 'vtgate' %}\ + --pprof=\"{{vitess_profile_mode}},path=/pprof/{{arewefastyet_exec_uuid}}/vtgate-{{ gateway.id }}\" \ + {% endif %}\ {{gateway.extra_flags|default("")}} \ {{extra_vtgate_flags|default("")}} \ " diff --git a/ansible/roles/vttablet/templates/vttablet.conf.j2 b/ansible/roles/vttablet/templates/vttablet.conf.j2 index 89f19b14..928e2e5c 100644 --- a/ansible/roles/vttablet/templates/vttablet.conf.j2 +++ b/ansible/roles/vttablet/templates/vttablet.conf.j2 @@ -44,11 +44,9 @@ EXTRA_VTTABLET_FLAGS="--alsologtostderr \ {% endif %} --grpc_max_message_size=67108864 \ --db_charset=utf8 \ - {% if pprof_targets is defined %} - {% for target in pprof_targets if target == "vttablet" %} - -pprof {{ pprof_args }},path=/tmp/pprof/vttablet-{{ tablet.id }},waitSig \ - {%- endfor %} - {% endif %} + {% if vitess_profile_binary == 'vttablet' %}\ + --pprof=\"{{vitess_profile_mode}},path=/pprof/{{arewefastyet_exec_uuid}}/vttablet-{{ tablet.id }}\" \ + {% endif %}\ {{ tablet.extra_flags | default("") }} \ {{ extra_vttablet_flags | default("") }} \ " diff --git a/go/admin/api.go b/go/admin/api.go index a8701acf..03702e0a 100644 --- a/go/admin/api.go +++ b/go/admin/api.go @@ -60,6 +60,9 @@ type ( SHA string `json:"sha"` Workloads []string `json:"workloads"` NumberOfExecutions string `json:"number_of_executions"` + EnableProfile bool `json:"enable_profile"` + BinaryToProfile string `json:"binary_to_profile"` + ProfileMode string `json:"profile_mode"` } clearQueueRequest struct { @@ -220,13 +223,23 @@ func (a *Admin) checkUserOrgMembership(client *goGithub.Client, username, orgNam } func (a *Admin) handleExecutionsAdd(c *gin.Context) { - source := c.PostForm("source") - sha := c.PostForm("sha") - workloads := c.PostFormArray("workloads") - numberOfExecutions := c.PostForm("numberOfExecutions") + requestPayload := executionRequest{ + Source: c.PostForm("source"), + SHA: c.PostForm("sha"), + Workloads: c.PostFormArray("workloads"), + NumberOfExecutions: c.PostForm("numberOfExecutions"), + EnableProfile: c.PostForm("enableProfiling") != "", + BinaryToProfile: c.PostForm("binaryToProfile"), + ProfileMode: c.PostForm("profileMode"), + } + + if requestPayload.Source == "" || requestPayload.SHA == "" || len(requestPayload.Workloads) == 0 || requestPayload.NumberOfExecutions == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "Missing required fields: Source, SHA, workflows, numberOfExecutions"}) + return + } - if source == "" || sha == "" || len(workloads) == 0 || numberOfExecutions == "" { - c.JSON(http.StatusBadRequest, gin.H{"error": "Missing required fields: Source and/or SHA"}) + if requestPayload.EnableProfile && (requestPayload.BinaryToProfile == "" || requestPayload.ProfileMode == "") { + c.JSON(http.StatusBadRequest, gin.H{"error": "When enabling profiling, please provide a binary to profile and a mode"}) return } @@ -248,7 +261,7 @@ func (a *Admin) handleExecutionsAdd(c *gin.Context) { return } - encryptedToken, err := server.Encrypt(token.AccessToken, a.auth) + requestPayload.Auth, err = server.Encrypt(token.AccessToken, a.auth) if err != nil { slog.Error("Failed to encrypt token: ", err) @@ -256,14 +269,6 @@ func (a *Admin) handleExecutionsAdd(c *gin.Context) { return } - requestPayload := executionRequest{ - Auth: encryptedToken, - Source: source, - SHA: sha, - Workloads: workloads, - NumberOfExecutions: numberOfExecutions, - } - jsonData, err := json.Marshal(requestPayload) if err != nil { diff --git a/go/admin/templates/add_new_executions.html b/go/admin/templates/add_new_executions.html index dd90cd5b..efe06cea 100644 --- a/go/admin/templates/add_new_executions.html +++ b/go/admin/templates/add_new_executions.html @@ -1,5 +1,6 @@
+
@@ -20,7 +21,8 @@ />
- +
+
+
+
+
+ +
+ +
+ + + +
\ No newline at end of file diff --git a/go/exec/exec.go b/go/exec/exec.go index 8757da89..322a3322 100644 --- a/go/exec/exec.go +++ b/go/exec/exec.go @@ -150,6 +150,13 @@ type Exec struct { vitessConfig vitessConfig vitessSchemaPath string + + ProfileInformation *ProfileInformation +} + +type ProfileInformation struct { + Binary string + Mode string } const ( @@ -395,6 +402,11 @@ func (e *Exec) prepareAnsibleForExecution() error { e.AnsibleConfig.AddExtraVar(ansible.KeyVitessSchema, e.vitessSchemaPath) e.vitessConfig.addToAnsible(&e.AnsibleConfig) + if e.ProfileInformation != nil { + e.AnsibleConfig.AddExtraVar(ansible.KeyProfileBinary, e.ProfileInformation.Binary) + e.AnsibleConfig.AddExtraVar(ansible.KeyProfileMode, e.ProfileInformation.Mode) + } + // runtime related values e.AnsibleConfig.AddExtraVar(ansible.KeyGoVersion, e.GolangVersion) diff --git a/go/infra/ansible/vars.go b/go/infra/ansible/vars.go index d7b9064f..357add5f 100644 --- a/go/infra/ansible/vars.go +++ b/go/infra/ansible/vars.go @@ -77,6 +77,12 @@ const ( // that will be used to execute the benchmark. KeyVtgatePlanner = "vitess_planner_version" + // KeyProfileBinary sets which binary we want to profile (vtgate or vttablet) + KeyProfileBinary = "vitess_profile_binary" + + // KeyProfileMode sets which mode of pprof we want to use. + KeyProfileMode = "vitess_profile_mode" + // KeyExtraFlagsVTGate represents the list of VTGate flag that will be passed down // to VTGate during startup. The flags are represented as follows in the string: // "--flag1 --flag2" diff --git a/go/server/api.go b/go/server/api.go index cf1cf3b4..d33ec5d1 100644 --- a/go/server/api.go +++ b/go/server/api.go @@ -469,7 +469,7 @@ func (s *Server) requestRun(c *gin.Context) { } // create execution element - elem := s.createSimpleExecutionQueueElement(cfg, "custom_run", sha, workload, string(macrobench.Gen4Planner), false, 0, currVersion) + elem := s.createSimpleExecutionQueueElement(cfg, "custom_run", sha, workload, string(macrobench.Gen4Planner), false, 0, currVersion, nil) // to new element to the queue s.addToQueue(elem) @@ -546,6 +546,9 @@ type ExecutionRequest struct { SHA string `json:"sha"` Workloads []string `json:"workloads"` NumberOfExecutions string `json:"number_of_executions"` + EnableProfile bool `json:"enable_profile-"` + BinaryToProfile string `json:"binary_to_profile"` + ProfileMode string `json:"profile_mode"` } func (s *Server) addExecutions(c *gin.Context) { @@ -576,9 +579,17 @@ func (s *Server) addExecutions(c *gin.Context) { } newElements := make([]*executionQueueElement, 0, execs*len(req.Workloads)) + var profileInformation *exec.ProfileInformation + if req.EnableProfile { + profileInformation = &exec.ProfileInformation{ + Binary: req.BinaryToProfile, + Mode: req.ProfileMode, + } + } + for _, workload := range req.Workloads { for i := 0; i < execs; i++ { - elem := s.createSimpleExecutionQueueElement(s.benchmarkConfig[strings.ToLower(workload)], req.Source, req.SHA, workload, string(macrobench.Gen4Planner), false, 0, git.Version{}) + elem := s.createSimpleExecutionQueueElement(s.benchmarkConfig[strings.ToLower(workload)], req.Source, req.SHA, workload, string(macrobench.Gen4Planner), false, 0, git.Version{}, profileInformation) elem.identifier.UUID = uuid.NewString() newElements = append(newElements, elem) } diff --git a/go/server/cron.go b/go/server/cron.go index 9c918eaa..cfd74f54 100644 --- a/go/server/cron.go +++ b/go/server/cron.go @@ -44,6 +44,7 @@ type ( PullBaseRef string Version git.Version UUID string + Profile *exec.ProfileInformation } executionQueue map[executionIdentifier]*executionQueueElement diff --git a/go/server/cron_execution.go b/go/server/cron_execution.go index 1a2eaa0c..623d291f 100644 --- a/go/server/cron_execution.go +++ b/go/server/cron_execution.go @@ -55,6 +55,7 @@ func (s *Server) executeSingle(config benchmarkConfig, identifier executionIdent e.PullBaseBranchRef = identifier.PullBaseRef e.VitessVersion = identifier.Version e.NextBenchmarkIsTheSame = nextIsSame + e.ProfileInformation = identifier.Profile e.RepoDir = s.getVitessPath() // Check if the previous benchmark is the same and if it is diff --git a/go/server/cron_handlers.go b/go/server/cron_handlers.go index 2035fdd0..330171e1 100644 --- a/go/server/cron_handlers.go +++ b/go/server/cron_handlers.go @@ -166,13 +166,13 @@ func (s *Server) createBranchElementWithComparisonOnPreviousAndRelease(config be // creating a benchmark for the latest commit on the branch with SourceCron as a source // this benchmark will be compared with the previous benchmark on branch with the given source, and with the latest release - newExecutionElement := s.createSimpleExecutionQueueElement(config, source, ref, workload, plannerVersion, false, 0, version) + newExecutionElement := s.createSimpleExecutionQueueElement(config, source, ref, workload, plannerVersion, false, 0, version, nil) elements = append(elements, newExecutionElement) if previousGitRef != "" { // creating an execution queue element for the latest benchmark with SourceCron as source // this will not be executed since the benchmark already exist, we still create the element in order to compare - previousElement := s.createSimpleExecutionQueueElement(config, source, previousGitRef, workload, plannerVersion, false, 0, version) + previousElement := s.createSimpleExecutionQueueElement(config, source, previousGitRef, workload, plannerVersion, false, 0, version, nil) previousElement.compareWith = append(previousElement.compareWith, newExecutionElement.identifier) newExecutionElement.compareWith = append(newExecutionElement.compareWith, previousElement.identifier) elements = append(elements, previousElement) @@ -181,7 +181,7 @@ func (s *Server) createBranchElementWithComparisonOnPreviousAndRelease(config be if lastRelease != nil && config.v.GetInt(keyMinimumVitessVersion) <= lastRelease.Version.Major { // creating an execution queue element for the latest release (comparing branch with the latest release) // this will probably not be executed the benchmark should already exist, we still create it to compare main once its benchmark is over - lastReleaseElement := s.createSimpleExecutionQueueElement(config, exec.SourceTag+lastRelease.Name, lastRelease.CommitHash, workload, plannerVersion, false, 0, lastRelease.Version) + lastReleaseElement := s.createSimpleExecutionQueueElement(config, exec.SourceTag+lastRelease.Name, lastRelease.CommitHash, workload, plannerVersion, false, 0, lastRelease.Version, nil) lastReleaseElement.compareWith = append(lastReleaseElement.compareWith, newExecutionElement.identifier) newExecutionElement.compareWith = append(newExecutionElement.compareWith, lastReleaseElement.identifier) elements = append(elements, lastReleaseElement) @@ -252,12 +252,12 @@ 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) + 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) + 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) @@ -296,11 +296,11 @@ func (s *Server) tagsCronHandler() { continue } if workload == "micro" { - elements = append(elements, s.createSimpleExecutionQueueElement(config, source, release.CommitHash, workload, "", true, 0, release.Version)) + elements = append(elements, s.createSimpleExecutionQueueElement(config, source, release.CommitHash, workload, "", true, 0, release.Version, nil)) } else { versions := git.GetPlannerVersions() for _, version := range versions { - elements = append(elements, s.createSimpleExecutionQueueElement(config, source, release.CommitHash, workload, string(version), true, 0, release.Version)) + elements = append(elements, s.createSimpleExecutionQueueElement(config, source, release.CommitHash, workload, string(version), true, 0, release.Version, nil)) } } } @@ -310,7 +310,7 @@ func (s *Server) tagsCronHandler() { } } -func (s *Server) createSimpleExecutionQueueElement(config benchmarkConfig, source, ref, workload, plannerVersion string, notify bool, pullNb int, version git.Version) *executionQueueElement { +func (s *Server) createSimpleExecutionQueueElement(config benchmarkConfig, source, ref, workload, plannerVersion string, notify bool, pullNb int, version git.Version, profileInformation *exec.ProfileInformation) *executionQueueElement { return &executionQueueElement{ config: config, retry: s.cronNbRetry, @@ -322,6 +322,7 @@ func (s *Server) createSimpleExecutionQueueElement(config benchmarkConfig, sourc PlannerVersion: plannerVersion, PullNb: pullNb, Version: version, + Profile: profileInformation, }, } } From 76cbb761084ba68e66f59cd1044518a165e90718 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 28 Oct 2024 17:52:47 -0600 Subject: [PATCH 03/12] wip disable all cron on prod Signed-off-by: Florent Poinsard --- config/prod/config.yaml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/config/prod/config.yaml b/config/prod/config.yaml index aebb1792..a1bcc056 100644 --- a/config/prod/config.yaml +++ b/config/prod/config.yaml @@ -1,8 +1,12 @@ exec-go-version: "1.22.0" -web-cron-schedule: "@midnight" -web-cron-schedule-pull-requests: "*/5 * * * *" -web-cron-schedule-tags: "*/1 * * * *" +#web-cron-schedule: "@midnight" +#web-cron-schedule-pull-requests: "*/5 * * * *" +#web-cron-schedule-tags: "*/1 * * * *" + +web-cron-schedule: "none" +web-cron-schedule-pull-requests: "none" +web-cron-schedule-tags: "none" web-port: 8080 web-benchmark-config-path: ./config/benchmarks/ From d3bbde3ae77de2dd6b825222b4cbb6428eae58b3 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 28 Oct 2024 18:03:52 -0600 Subject: [PATCH 04/12] fix typo in struct field name Signed-off-by: Florent Poinsard --- go/server/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/server/api.go b/go/server/api.go index d33ec5d1..8763e633 100644 --- a/go/server/api.go +++ b/go/server/api.go @@ -546,7 +546,7 @@ type ExecutionRequest struct { SHA string `json:"sha"` Workloads []string `json:"workloads"` NumberOfExecutions string `json:"number_of_executions"` - EnableProfile bool `json:"enable_profile-"` + EnableProfile bool `json:"enable_profile"` BinaryToProfile string `json:"binary_to_profile"` ProfileMode string `json:"profile_mode"` } From 4b6218dd97f426cabd2cbb15e61b15f4e24c98b5 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Mon, 28 Oct 2024 18:20:15 -0600 Subject: [PATCH 05/12] fix issue with vitess version Signed-off-by: Florent Poinsard --- ansible/roles/vttablet/templates/vttablet.conf.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible/roles/vttablet/templates/vttablet.conf.j2 b/ansible/roles/vttablet/templates/vttablet.conf.j2 index 928e2e5c..8e9d0253 100644 --- a/ansible/roles/vttablet/templates/vttablet.conf.j2 +++ b/ansible/roles/vttablet/templates/vttablet.conf.j2 @@ -33,7 +33,7 @@ EXTRA_VTTABLET_FLAGS="--alsologtostderr \ --queryserver-config-pool-size={{ tablet.connection_pool_size | default(vttablet_connection_pool_size) }} \ --queryserver-config-stream-pool-size={{ tablet.stream_pool_size | default(vttablet_stream_pool_size) }} \ --queryserver-config-transaction-cap={{ tablet.transaction_cap | default(vttablet_transaction_cap) }} \ - {% if vitess_major_version <= 18 %} + {% if vitess_major_version > 1 and vitess_major_version <= 18 %} --queryserver-config-query-timeout=900 \ --queryserver-config-schema-reload-time=60 \ --queryserver-config-transaction-timeout=300 \ From fe596687b52806e57d8dc7e3eba01514431f0d8c Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 29 Oct 2024 14:15:07 -0600 Subject: [PATCH 06/12] Proper detection of next_is_same Signed-off-by: Florent Poinsard --- go/server/cron_execution.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/server/cron_execution.go b/go/server/cron_execution.go index 623d291f..13ca926e 100644 --- a/go/server/cron_execution.go +++ b/go/server/cron_execution.go @@ -223,7 +223,7 @@ func (s *Server) cronExecutionQueueWatcher() { if element.Executing { continue } - if element.identifier.equalWithoutUUID(nextExecuteElement.identifier) { + if nextExecuteElement.identifier.UUID != element.identifier.UUID && element.identifier.equalWithoutUUID(nextExecuteElement.identifier) { nextBenchmarkIsTheSame = true break } From 4586f67bba7243493f4c15cc6bf3117d314da642 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 29 Oct 2024 14:35:24 -0600 Subject: [PATCH 07/12] Start and stop pprof on sig Signed-off-by: Florent Poinsard --- ansible/macrobench.yml | 10 ++++++++++ ansible/roles/profile/tasks/start.yml | 16 ++++++++++++++++ ansible/roles/profile/tasks/stop.yml | 16 ++++++++++++++++ ansible/roles/vtgate/templates/vtgate.conf.j2 | 2 +- .../roles/vttablet/templates/vttablet.conf.j2 | 2 +- 5 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 ansible/roles/profile/tasks/start.yml create mode 100644 ansible/roles/profile/tasks/stop.yml diff --git a/ansible/macrobench.yml b/ansible/macrobench.yml index 3cf54797..fde8de4e 100644 --- a/ansible/macrobench.yml +++ b/ansible/macrobench.yml @@ -39,10 +39,20 @@ name: sysbench tasks_from: install +- hosts: macrobench + include_role: + name: profile + tasks_from: start + - hosts: macrobench roles: - macrobench +- hosts: macrobench + include_role: + name: profile + tasks_from: stop + - name: Clean Post Macrobench when: arewefastyet_next_exec_is_same is undefined import_playbook: clean_macrobench.yml diff --git a/ansible/roles/profile/tasks/start.yml b/ansible/roles/profile/tasks/start.yml new file mode 100644 index 00000000..d4fbfcd3 --- /dev/null +++ b/ansible/roles/profile/tasks/start.yml @@ -0,0 +1,16 @@ +# Copyright 2024 The Vitess Authors. +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +--- +- name: Start profiling + shell: | + kill -s USR1 $(pgrep -d " " {{vitess_profile_binary}}) + when: vitess_profile_binary is defined \ No newline at end of file diff --git a/ansible/roles/profile/tasks/stop.yml b/ansible/roles/profile/tasks/stop.yml new file mode 100644 index 00000000..8eabe183 --- /dev/null +++ b/ansible/roles/profile/tasks/stop.yml @@ -0,0 +1,16 @@ +# Copyright 2024 The Vitess Authors. +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +--- +- name: Stop profiling + shell: | + kill -s USR1 $(pgrep -d " " {{vitess_profile_binary}}) + when: vitess_profile_binary is defined \ No newline at end of file diff --git a/ansible/roles/vtgate/templates/vtgate.conf.j2 b/ansible/roles/vtgate/templates/vtgate.conf.j2 index 5a4e49e3..709568cf 100644 --- a/ansible/roles/vtgate/templates/vtgate.conf.j2 +++ b/ansible/roles/vtgate/templates/vtgate.conf.j2 @@ -23,7 +23,7 @@ EXTRA_VTGATE_FLAGS="\ --vtgate-config-terse-errors \ --tablet_types_to_wait REPLICA \ {% if vitess_profile_binary == 'vtgate' %}\ - --pprof=\"{{vitess_profile_mode}},path=/pprof/{{arewefastyet_exec_uuid}}/vtgate-{{ gateway.id }}\" \ + --pprof=\"{{vitess_profile_mode}},waitSig,path=/pprof/{{arewefastyet_exec_uuid}}/vtgate-{{ gateway.id }}\" \ {% endif %}\ {{gateway.extra_flags|default("")}} \ {{extra_vtgate_flags|default("")}} \ diff --git a/ansible/roles/vttablet/templates/vttablet.conf.j2 b/ansible/roles/vttablet/templates/vttablet.conf.j2 index 8e9d0253..592866ea 100644 --- a/ansible/roles/vttablet/templates/vttablet.conf.j2 +++ b/ansible/roles/vttablet/templates/vttablet.conf.j2 @@ -45,7 +45,7 @@ EXTRA_VTTABLET_FLAGS="--alsologtostderr \ --grpc_max_message_size=67108864 \ --db_charset=utf8 \ {% if vitess_profile_binary == 'vttablet' %}\ - --pprof=\"{{vitess_profile_mode}},path=/pprof/{{arewefastyet_exec_uuid}}/vttablet-{{ tablet.id }}\" \ + --pprof=\"{{vitess_profile_mode}},waitSig,path=/pprof/{{arewefastyet_exec_uuid}}/vttablet-{{ tablet.id }}\" \ {% endif %}\ {{ tablet.extra_flags | default("") }} \ {{ extra_vttablet_flags | default("") }} \ From d37a9996acb747568aae0d1e2b498a2b03bc7661 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 29 Oct 2024 14:42:00 -0600 Subject: [PATCH 08/12] Simplify the profile role in ansible Signed-off-by: Florent Poinsard --- ansible/macrobench.yml | 10 ++++------ .../roles/profile/tasks/{start.yml => main.yml} | 2 +- ansible/roles/profile/tasks/stop.yml | 16 ---------------- 3 files changed, 5 insertions(+), 23 deletions(-) rename ansible/roles/profile/tasks/{start.yml => main.yml} (96%) delete mode 100644 ansible/roles/profile/tasks/stop.yml diff --git a/ansible/macrobench.yml b/ansible/macrobench.yml index fde8de4e..44b6ab32 100644 --- a/ansible/macrobench.yml +++ b/ansible/macrobench.yml @@ -40,18 +40,16 @@ tasks_from: install - hosts: macrobench - include_role: - name: profile - tasks_from: start + roles: + - profile - hosts: macrobench roles: - macrobench - hosts: macrobench - include_role: - name: profile - tasks_from: stop + roles: + - profile - name: Clean Post Macrobench when: arewefastyet_next_exec_is_same is undefined diff --git a/ansible/roles/profile/tasks/start.yml b/ansible/roles/profile/tasks/main.yml similarity index 96% rename from ansible/roles/profile/tasks/start.yml rename to ansible/roles/profile/tasks/main.yml index d4fbfcd3..6a403caf 100644 --- a/ansible/roles/profile/tasks/start.yml +++ b/ansible/roles/profile/tasks/main.yml @@ -10,7 +10,7 @@ # limitations under the License. --- -- name: Start profiling +- name: Toggle profiling shell: | kill -s USR1 $(pgrep -d " " {{vitess_profile_binary}}) when: vitess_profile_binary is defined \ No newline at end of file diff --git a/ansible/roles/profile/tasks/stop.yml b/ansible/roles/profile/tasks/stop.yml deleted file mode 100644 index 8eabe183..00000000 --- a/ansible/roles/profile/tasks/stop.yml +++ /dev/null @@ -1,16 +0,0 @@ -# Copyright 2024 The Vitess Authors. -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# http://www.apache.org/licenses/LICENSE-2.0 -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - ---- -- name: Stop profiling - shell: | - kill -s USR1 $(pgrep -d " " {{vitess_profile_binary}}) - when: vitess_profile_binary is defined \ No newline at end of file From 130c984f629490619d0415e4f6e0538a6328bdb3 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 29 Oct 2024 15:11:27 -0600 Subject: [PATCH 09/12] Fix data race and execution ID comparison Signed-off-by: Florent Poinsard --- go/server/api.go | 7 ++++++- go/server/cron.go | 13 +++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/go/server/api.go b/go/server/api.go index 8763e633..a425bd06 100644 --- a/go/server/api.go +++ b/go/server/api.go @@ -595,7 +595,12 @@ func (s *Server) addExecutions(c *gin.Context) { } } - s.appendToQueue(newElements) + addToQueue := func() { + mtx.Lock() + defer mtx.Unlock() + s.appendToQueue(newElements) + } + addToQueue() c.JSON(http.StatusCreated, "") } diff --git a/go/server/cron.go b/go/server/cron.go index cfd74f54..c5bcd30e 100644 --- a/go/server/cron.go +++ b/go/server/cron.go @@ -64,6 +64,19 @@ var ( func (ei executionIdentifier) equalWithoutUUID(id executionIdentifier) bool { ei.UUID = "" id.UUID = "" + + // If one has a profile and the other doesn't, it's not equal + if ei.Profile != nil && id.Profile == nil || ei.Profile == nil && id.Profile != nil { + return false + } + // If both have a profile, but they're different, it's not equal + if ei.Profile != nil && id.Profile != nil && *ei.Profile != *id.Profile { + return false + } + // Setting the profiles to nil so they don't mess up the equality comparison below + ei.Profile = nil + id.Profile = nil + return ei == id } From 564b3a5b8857e19ec5f8b281c46b56e335098363 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 29 Oct 2024 15:16:46 -0600 Subject: [PATCH 10/12] UI tweaks Signed-off-by: Florent Poinsard --- go/admin/templates/add_new_executions.html | 42 ++++++++++++---------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/go/admin/templates/add_new_executions.html b/go/admin/templates/add_new_executions.html index efe06cea..4029e8c1 100644 --- a/go/admin/templates/add_new_executions.html +++ b/go/admin/templates/add_new_executions.html @@ -1,25 +1,24 @@
-
-
- - -
-
- - -
+
+ + +
+
+
+ +

@@ -153,6 +152,7 @@ hx-target="#response" class="bg-green-500 hover:green text-white font-bold py-2 px-4 rounded focus:outline-none focus:shadow-outline" type="submit" + onclick="clearHelperText()" > Add Execution @@ -177,6 +177,10 @@ } }); + function clearHelperText() { + document.getElementById("response").innerHTML = ""; + } + function toggleProfilingFields() { const checkbox = document.getElementById('enableProfilingCheckbox'); const profilingFields = document.getElementById('profilingFields'); From 5b10dec361858e0ed9cdad3ec376ae2d18d126fd Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 29 Oct 2024 15:18:51 -0600 Subject: [PATCH 11/12] Self review Signed-off-by: Florent Poinsard --- ansible/roles/vtgate/templates/vtgate.conf.j2 | 4 ++-- config/prod/config.yaml | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/ansible/roles/vtgate/templates/vtgate.conf.j2 b/ansible/roles/vtgate/templates/vtgate.conf.j2 index 709568cf..5c07b985 100644 --- a/ansible/roles/vtgate/templates/vtgate.conf.j2 +++ b/ansible/roles/vtgate/templates/vtgate.conf.j2 @@ -23,8 +23,8 @@ EXTRA_VTGATE_FLAGS="\ --vtgate-config-terse-errors \ --tablet_types_to_wait REPLICA \ {% if vitess_profile_binary == 'vtgate' %}\ - --pprof=\"{{vitess_profile_mode}},waitSig,path=/pprof/{{arewefastyet_exec_uuid}}/vtgate-{{ gateway.id }}\" \ - {% endif %}\ + --pprof=\"{{vitess_profile_mode}},waitSig,path=/pprof/{{arewefastyet_exec_uuid}}/vtgate-{{ gateway.id }}\" \ + {% endif %}\ {{gateway.extra_flags|default("")}} \ {{extra_vtgate_flags|default("")}} \ " diff --git a/config/prod/config.yaml b/config/prod/config.yaml index a1bcc056..aebb1792 100644 --- a/config/prod/config.yaml +++ b/config/prod/config.yaml @@ -1,12 +1,8 @@ exec-go-version: "1.22.0" -#web-cron-schedule: "@midnight" -#web-cron-schedule-pull-requests: "*/5 * * * *" -#web-cron-schedule-tags: "*/1 * * * *" - -web-cron-schedule: "none" -web-cron-schedule-pull-requests: "none" -web-cron-schedule-tags: "none" +web-cron-schedule: "@midnight" +web-cron-schedule-pull-requests: "*/5 * * * *" +web-cron-schedule-tags: "*/1 * * * *" web-port: 8080 web-benchmark-config-path: ./config/benchmarks/ From 724acdb0cf737ec3975ae213ab8b38b6fbfeeaae Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 29 Oct 2024 15:39:48 -0600 Subject: [PATCH 12/12] tweak ui Signed-off-by: Florent Poinsard --- go/admin/templates/add_new_executions.html | 27 +++++++++++++++------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/go/admin/templates/add_new_executions.html b/go/admin/templates/add_new_executions.html index 4029e8c1..43f3d77d 100644 --- a/go/admin/templates/add_new_executions.html +++ b/go/admin/templates/add_new_executions.html @@ -131,21 +131,32 @@ -