Merge branch 'main' into lint_workflow

Signed-off-by: Dhairya Majmudar <[email protected]>
vitessio · Aug 6, 2024 · 8bab009 · 8bab009
2 parents b25ff8f + e964b37
commit 8bab009
Show file tree

Hide file tree

Showing 10 changed files with 136 additions and 71 deletions.
diff --git a/go/tools/macrobench/results.go b/go/tools/macrobench/results.go
@@ -218,7 +218,7 @@ func Compare(client storage.SQLClient, old, new string, workloads []string, plan
 				return
 			}
 
-			if len(oldResult.Results) == 0 && len(newResult.Results) == 0 {
+			if len(oldResult.Results) == 0 || len(newResult.Results) == 0 {
 				mu.Lock()
 				defer mu.Unlock()
 				results[workload] = StatisticalCompareResults{
@@ -230,6 +230,7 @@ func Compare(client storage.SQLClient, old, new string, workloads []string, plan
 						"vtgate":   {},
 						"vttablet": {},
 					},
+					MissingResults: true,
 				}
 				return
 			}
@@ -259,7 +260,7 @@ func CompareFKs(client storage.SQLClient, oldWorkload, newWorkload string, sha s
 		return StatisticalCompareResults{}, err
 	}
 
-	if len(oldResult.Results) == 0 && len(newResult.Results) == 0 {
+	if len(oldResult.Results) == 0 || len(newResult.Results) == 0 {
 		return StatisticalCompareResults{
 			ComponentsCPUTime: map[string]StatisticalResult{
 				"vtgate":   {},
@@ -269,6 +270,7 @@ func CompareFKs(client storage.SQLClient, oldWorkload, newWorkload string, sha s
 				"vtgate":   {},
 				"vttablet": {},
 			},
+			MissingResults: true,
 		}, nil
 	}
 

diff --git a/go/tools/macrobench/stats.go b/go/tools/macrobench/stats.go
@@ -87,6 +87,7 @@ type (
 
 		TotalComponentsMemStatsAllocBytes StatisticalResult            `json:"total_components_mem_stats_alloc_bytes"`
 		ComponentsMemStatsAllocBytes      map[string]StatisticalResult `json:"components_mem_stats_alloc_bytes"`
+		MissingResults                    bool                         `json:"missing_results"`
 	}
 )
 

diff --git a/website/src/common/CompareActions.tsx b/website/src/common/CompareActions.tsx
@@ -17,7 +17,7 @@ limitations under the License.
 import { Button } from "@/components/ui/button";
 import { cn } from "@/library/utils";
 import { VitessRefs } from "@/types";
-import React from "react";
+import React, { useEffect } from "react";
 import VitessRefsCommand from "./VitessRefsCommand";
 
 export type CompareActionsProps = {
@@ -47,6 +47,19 @@ export default function CompareAction(props: CompareActionsProps) {
   } = props;
   const isButtonDisabled = !oldGitRef || !newGitRef;
 
+  useEffect(() => {
+    const handleKeyDown = (event: KeyboardEvent) => {
+      if (event.key === "Enter" && !isButtonDisabled) {
+        compareClicked();
+      }
+    };
+
+    document.addEventListener("keydown", handleKeyDown);
+    return () => {
+      document.removeEventListener("keydown", handleKeyDown);
+    };
+  }, [compareClicked, isButtonDisabled]);
+
   return (
     <div className={cn("flex flex-col md:flex-row gap-4", className)}>
       <div className="flex flex-col">

diff --git a/website/src/common/MacroBenchmarkTable.tsx b/website/src/common/MacroBenchmarkTable.tsx
@@ -29,11 +29,11 @@ import {
   TooltipProvider,
   TooltipTrigger,
 } from "@/components/ui/tooltip";
-import { MacroBenchmarkTableData, Range } from "@/types";
+import { MacroBenchmarkTableData, Range, VitessRefs } from "@/types";
 import {
   fixed,
   formatByte,
-  formatGitRef,
+  getRefName,
   secondToMicrosecond,
 } from "@/utils/Utils";
 import { Link } from "react-router-dom";
@@ -43,6 +43,7 @@ export type MacroBenchmarkTableProps = {
   old: string;
   new: string;
   isGitRef?: boolean;
+  vitessRefs: VitessRefs | null;
 };
 
 const getDeltaBadgeVariant = (key: string, delta: number, p: number) => {
@@ -92,9 +93,10 @@ const getValue = <T, K extends keyof T>(obj: T, key: K): T[K] => obj[key];
 
 export default function MacroBenchmarkTable({
   data,
-  new: newColumn,
+  new: newGitRef,
   old,
   isGitRef = true,
+  vitessRefs,
 }: MacroBenchmarkTableProps) {
   if (!data) {
     return null;
@@ -126,27 +128,27 @@ export default function MacroBenchmarkTable({
         <TableRow className="hover:bg-background border-b">
           <TableHead className="w-[200px]"></TableHead>
           <TableHead className="text-center text-primary font-semibold min-w-[150px]">
-            {isGitRef ? (
+            {isGitRef && vitessRefs ? (
               <Link
                 to={`https://github.com/vitessio/vitess/commit/${old}`}
                 target="__blank"
               >
-                {formatGitRef(old) || "N/A"}
+                {getRefName(old, vitessRefs) || "N/A"}
               </Link>
             ) : (
               <>{old}</>
             )}
           </TableHead>
           <TableHead className="text-center text-primary font-semibold min-w-[150px]">
-            {isGitRef ? (
+            {isGitRef && vitessRefs ? (
               <Link
-                to={`https://github.com/vitessio/vitess/commit/${newColumn}`}
+                to={`https://github.com/vitessio/vitess/commit/${newGitRef}`}
                 target="__blank"
               >
-                {formatGitRef(newColumn) || "N/A"}
+                {getRefName(newGitRef, vitessRefs) || "N/A"}
               </Link>
             ) : (
-              <>{newColumn}</>
+              <>{newGitRef}</>
             )}
           </TableHead>
           <TableHead className="lg:w-[150px] text-center font-semibold">

diff --git a/website/src/pages/ComparePage/ComparePage.tsx b/website/src/pages/ComparePage/ComparePage.tsx
@@ -18,38 +18,50 @@ import MacroBenchmarkTable from "@/common/MacroBenchmarkTable";
 import { Button } from "@/components/ui/button";
 import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card";
 import { Skeleton } from "@/components/ui/skeleton";
-import { CompareData, MacroBenchmarkTableData } from "@/types";
+import { CompareData, MacroBenchmarkTableData, VitessRefs } from "@/types";
 import useApiCall from "@/utils/Hook";
-import { formatCompareData } from "@/utils/Utils";
+import { formatCompareData, getRefName } from "@/utils/Utils";
 import { PlusCircledIcon } from "@radix-ui/react-icons";
 import { useEffect, useState } from "react";
 import { Link, useNavigate } from "react-router-dom";
 import CompareHero from "./components/CompareHero";
 
 export default function Compare() {
+  const navigate = useNavigate();
   const urlParams = new URLSearchParams(window.location.search);
 
   const [gitRef, setGitRef] = useState({
     old: urlParams.get("old") || "",
     new: urlParams.get("new") || "",
   });
 
-  const navigate = useNavigate();
-
-  useEffect(() => {
-    navigate(`?old=${gitRef.old}&new=${gitRef.new}`);
-  }, [gitRef.old, gitRef.new]);
-
   const {
     data: data,
     isLoading: isMacrobenchLoading,
     error: macrobenchError,
   } = useApiCall<CompareData[]>(
-    `${import.meta.env.VITE_API_URL}macrobench/compare?new=${gitRef.new}&old=${
-      gitRef.old
-    }`,
+    gitRef.old && gitRef.new
+      ? `${import.meta.env.VITE_API_URL}macrobench/compare?new=${
+          gitRef.new
+        }&old=${gitRef.old}`
+      : ``
   );
 
+  const { data: vitessRefs } = useApiCall<VitessRefs>(
+    `${import.meta.env.VITE_API_URL}vitess/refs`
+  );
+
+  useEffect(() => {
+    let oldRefName = gitRef.old;
+    let newRefName = gitRef.new;
+    if (vitessRefs) {
+      oldRefName = getRefName(gitRef.old, vitessRefs);
+      newRefName = getRefName(gitRef.new, vitessRefs);
+    }
+
+    navigate(`?old=${oldRefName}&new=${newRefName}`);
+  }, [gitRef.old, gitRef.new, vitessRefs]);
+
   let formattedData: MacroBenchmarkTableData[] = [];
 
   if (data !== null && data.length > 0) {
@@ -58,7 +70,11 @@ export default function Compare() {
 
   return (
     <>
-      <CompareHero gitRef={gitRef} setGitRef={setGitRef} />
+      <CompareHero
+        gitRef={gitRef}
+        setGitRef={setGitRef}
+        vitessRefs={vitessRefs}
+      />
       {macrobenchError && (
         <div className="text-red-500 text-center my-2">{macrobenchError}</div>
       )}
@@ -75,6 +91,11 @@ export default function Compare() {
             })}
           </>
         )}
+        {!isMacrobenchLoading && data === null && (
+          <div className="md:text-xl text-primary">
+            Chose two commits to compare
+          </div>
+        )}
         {!isMacrobenchLoading && data !== null && data.length > 0 && (
           <>
             {data.map((macro, index) => {
@@ -89,6 +110,7 @@ export default function Compare() {
                         variant="outline"
                         size="sm"
                         className="h-8 w-fit border-dashed mt-4 md:mt-0"
+                        disabled={macro.result.missing_results}
                       >
                         <PlusCircledIcon className="mr-2 h-4 w-4 text-primary" />
                         <Link
@@ -99,11 +121,18 @@ export default function Compare() {
                       </Button>
                     </CardHeader>
                     <CardContent className="w-full p-0">
-                      <MacroBenchmarkTable
-                        data={formattedData[index]}
-                        new={gitRef.new}
-                        old={gitRef.old}
-                      />
+                      {macro.result.missing_results ? (
+                        <div className="text-center md:text-xl text-destructive pb-12">
+                          Missing results for this workload
+                        </div>
+                      ) : (
+                        <MacroBenchmarkTable
+                          data={formattedData[index]}
+                          new={gitRef.new}
+                          old={gitRef.old}
+                          vitessRefs={vitessRefs}
+                        />
+                      )}
                     </CardContent>
                   </Card>
                 </div>

diff --git a/website/src/pages/ComparePage/components/CompareHero.tsx b/website/src/pages/ComparePage/components/CompareHero.tsx
@@ -32,23 +32,18 @@ export type CompareHeroProps = {
       new: string;
     }>
   >;
+  vitessRefs: VitessRefs | null;
 };
 
 export default function CompareHero(props: CompareHeroProps) {
-  const { gitRef, setGitRef } = props;
+  const { gitRef, setGitRef, vitessRefs } = props;
   const [oldGitRef, setOldGitRef] = useState(gitRef.old);
   const [newGitRef, setNewGitRef] = useState(gitRef.new);
 
-  const { data: vitessRefs } = useApiCall<VitessRefs>(
-    `${import.meta.env.VITE_API_URL}vitess/refs`,
-  );
-
   const compareClicked = () => {
     setGitRef({ old: oldGitRef, new: newGitRef });
   };
 
-  useEffect(() => {}, [vitessRefs]);
-
   useEffect(() => {
     setOldGitRef(gitRef.old);
     setNewGitRef(gitRef.new);

diff --git a/website/src/pages/ForeignKeysPage/ForeignKeysPage.tsx b/website/src/pages/ForeignKeysPage/ForeignKeysPage.tsx
@@ -23,25 +23,10 @@ import { Button } from "@/components/ui/button";
 import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card";
 import { Skeleton } from "@/components/ui/skeleton";
 import useApiCall from "@/utils/Hook";
-import { errorApi, formatCompareResult, formatGitRef } from "@/utils/Utils";
+import { formatCompareResult, getRefName } from "@/utils/Utils";
 import { PlusCircledIcon } from "@radix-ui/react-icons";
 import ForeignKeysHero from "./components/ForeignKeysHero";
 
-export const formatTitle = (gitRef: string, vitessRefs: VitessRefs): string => {
-  let title = formatGitRef(gitRef);
-  vitessRefs.branches.forEach((branch) => {
-    if (branch.commit_hash.match(gitRef)) {
-      title = branch.name;
-    }
-  });
-  vitessRefs.tags.forEach((branch) => {
-    if (branch.commit_hash.match(gitRef)) {
-      title = branch.name;
-    }
-  });
-  return title;
-};
-
 export default function ForeignKeys() {
   const urlParams = new URLSearchParams(window.location.search);
 
@@ -60,9 +45,11 @@ export default function ForeignKeys() {
     isLoading: isMacrobenchLoading,
     error: macrobenchError,
   } = useApiCall<CompareResult>(
-    `${import.meta.env.VITE_API_URL}fk/compare?sha=${gitRef}&newWorkload=${
-      workload.new
-    }&oldWorkload=${workload.old}`,
+    gitRef && workload.old && workload.new
+      ? `${import.meta.env.VITE_API_URL}fk/compare?sha=${gitRef}&newWorkload=${
+          workload.new
+        }&oldWorkload=${workload.old}`
+      : ""
   );
 
   let formattedData = data !== null ? formatCompareResult(data) : null;
@@ -87,15 +74,17 @@ export default function ForeignKeys() {
         />
       )}
 
-      {macrobenchError && (
-        <div className="text-red-500 text-center my-2">{macrobenchError}</div>
-      )}
+      <section className="flex flex-col items-center">
+        {macrobenchError && (
+          <div className="text-destructive">{macrobenchError}</div>
+        )}
 
-      {(formattedData === null || data === null) && !isMacrobenchLoading && (
-        <div className="text-red-500 text-center my-2">{errorApi}</div>
-      )}
+        {!isMacrobenchLoading && data === null && (
+          <div className="md:text-xl text-primary">
+            Chose two commits to compare
+          </div>
+        )}
 
-      <section className="flex flex-col items-center">
         {isMacrobenchLoading && (
           <>
             {[...Array(8)].map((_, index) => {
@@ -123,7 +112,7 @@ export default function ForeignKeys() {
                           to={`https://github.com/vitessio/vitess/commit/${gitRef}`}
                           target="__blank"
                         >
-                          {formatTitle(gitRef, vitessRefs)}
+                          {getRefName(gitRef, vitessRefs)}
                         </Link>
                       )}
                     </CardTitle>
@@ -141,12 +130,19 @@ export default function ForeignKeys() {
                     </Button>
                   </CardHeader>
                   <CardContent className="w-full p-0">
-                    <MacroBenchmarkTable
-                      data={formattedData}
-                      new={workload.new}
-                      old={workload.old}
-                      isGitRef={false}
-                    />
+                    {data.missing_results ? (
+                      <div className="text-center md:text-xl text-destructive pb-12">
+                        Missing results for this workloads
+                      </div>
+                    ) : (
+                      <MacroBenchmarkTable
+                        data={formattedData}
+                        new={workload.new}
+                        old={workload.old}
+                        isGitRef={false}
+                        vitessRefs={null}
+                      />
+                    )}
                   </CardContent>
                 </Card>
               </div>

diff --git a/website/src/types/index.ts b/website/src/types/index.ts
@@ -111,6 +111,7 @@ export interface CompareResult {
     vtgate: ComparedValue;
     vttablet: ComparedValue;
   };
+  missing_results: boolean;
 }
 
 export interface CompareData {
-Original file line number
+Diff line change
@@ Expand Up / @@ -87,6 +87,7 @@ type ( @@
     		TotalComponentsMemStatsAllocBytes StatisticalResult            `json:"total_components_mem_stats_alloc_bytes"`
     		ComponentsMemStatsAllocBytes      map[string]StatisticalResult `json:"components_mem_stats_alloc_bytes"`
+    		MissingResults                    bool                         `json:"missing_results"`
     	}
     )
@@ Expand Down @@