reduce calls to jira issue status (#2610)

* reduce calls to jira issue status

src/config/feature-flags.ts

@@ -41,7 +41,8 @@ export enum NumberFlags {
 	PREEMPTIVE_RATE_LIMIT_THRESHOLD = "preemptive-rate-limit-threshold",
 	NUMBER_OF_BUILD_PAGES_TO_FETCH_IN_PARALLEL = "number-of-build-to-fetch-in-parallel",
 	BACKFILL_PAGE_SIZE = "backfill-page-size",
-	INSTALLATION_TOKEN_CACHE_MAX_SIZE = "installation-token-cache-max-size"
+	INSTALLATION_TOKEN_CACHE_MAX_SIZE = "installation-token-cache-max-size",
+	SKIP_PROCESS_QUEUE_IF_ISSUE_NOT_FOUND_TIMEOUT = "skip-process-queue-when-issue-not-exists-timeout"
 }
 
 const createLaunchdarklyUser = (key?: string): LDUser => {

src/transforms/push.test.ts

@@ -2,7 +2,7 @@ import { enqueuePush, processPush } from "./push";
 import { sqsQueues } from "../sqs/queues";
 import { when } from "jest-when";
 import { GitHubCommit, GitHubRepository } from "interfaces/github";
-import { shouldSendAll, booleanFlag, BooleanFlags } from "config/feature-flags";
+import { shouldSendAll, booleanFlag, BooleanFlags, numberFlag, NumberFlags } from "config/feature-flags";
 import { getLogger } from "config/logger";
 import { GitHubInstallationClient } from "../github/client/github-installation-client";
 import { DatabaseStateCreator, CreatorResult } from "test/utils/database-state-creator";
@@ -115,6 +115,8 @@ describe("Enqueue push", () => {
 				.forCloud()
 				.create();
 			when(shouldSendAll).calledWith("commits", expect.anything(), expect.anything()).mockResolvedValue(false);
+			when(numberFlag).calledWith(NumberFlags.SKIP_PROCESS_QUEUE_IF_ISSUE_NOT_FOUND_TIMEOUT, expect.anything(), expect.anything())
+				.mockResolvedValue(10000);
 			issueKey = `KEY-${new Date().getTime()}`;
 			sha = `sha-${issueKey}`;
 		});

src/transforms/push.ts

@@ -1,4 +1,5 @@
 import Logger from "bunyan";
+import { uniq } from "lodash";
 import { Subscription } from "models/subscription";
 import { getJiraClient, JiraClient } from "../jira/client/jira-client";
 import { JiraClientError } from "../jira/client/axios";
@@ -140,11 +141,14 @@ export const processPush = async (github: GitHubInstallationClient, payload: Pus
 		}
 
 		const recentShas = shas.slice(0, MAX_COMMIT_HISTORY);
+
+		const invalidIssueKeys = await tryGetInvalidIssueKeys(recentShas, subscription, jiraClient, log);
+
 		const commitPromises: Promise<JiraCommit | null>[] = recentShas.map(async (sha): Promise<JiraCommit | null> => {
 			try {
 
 				if (await booleanFlag(BooleanFlags.SKIP_PROCESS_QUEUE_IF_ISSUE_NOT_FOUND, jiraHost)) {
-					if (!await someIssueKeysExistsOnJira(subscription.jiraHost, sha.issueKeys, jiraClient, log)) {
+					if (sha.issueKeys.every(k => invalidIssueKeys.includes(k))) {
 						log.info("Issue key not found on jira, skip processing commits");
 						return null;
 					}
@@ -245,36 +249,52 @@ export const processPush = async (github: GitHubInstallationClient, payload: Pus
 	}
 };
 
-const someIssueKeysExistsOnJira = async (jiraHost: string, issueKeys: string[], jiraClient: JiraClient, log: Logger): Promise<boolean> => {
-	for (const issueKey of issueKeys) {
-		try {
-			const status = await getIssueStatusFromRedis(jiraHost, issueKey);
-			if (status === "not_exist") {
-				continue;
-			} else if (status === "exist") {
-				return true;
-			}
-			await jiraClient.issues.get(issueKey);
-			await saveIssueStatusToRedis(jiraHost, issueKey, "exist");
-			return true;
-		} catch (e) {
-			if (e instanceof JiraClientError) {
-				if (e.status !== 404) {
-					//some other jira client error happen,
-					//return true to continue processing the msg for the safe side
-					log.warn("Found other errors status when fetching issue status", { err: e });
-					return true;
-				} else {
-					await saveIssueStatusToRedis(jiraHost, issueKey, "not_exist");
+const tryGetInvalidIssueKeys = async(
+	recentShas: PushQueueMessagePayload["shas"],
+	subscription: Subscription,
+	jiraClient: JiraClient,
+	log: Logger
+): Promise<string[]> => {
+	try {
+		const invalidIssueKeys: string[] = [];
+		if (await booleanFlag(BooleanFlags.SKIP_PROCESS_QUEUE_IF_ISSUE_NOT_FOUND, subscription.jiraHost)) {
+			const issueKeys = uniq(recentShas.flatMap(sha => sha.issueKeys));
+			for (const issueKey of issueKeys) {
+				const status = await processIssueKeyStatusFromJiraApi(subscription.jiraHost, issueKey, jiraClient, log);
+				if (status === "not_exist") {
+					invalidIssueKeys.push(issueKey);
 				}
-			} else {
-				//some unknow error,
-				//return true to continue processing the msg for the safe side
-				log.warn("Found other errors when fetching issue status", { err: e });
-				return true;
 			}
 		}
+		return invalidIssueKeys;
+	} catch (e) {
+		log.error({ err: e }, "Found some error when trying to get invalid issue keys");
+		return [];
+	}
+};
+
+const processIssueKeyStatusFromJiraApi = async (jiraHost: string, issueKey: string, jiraClient: JiraClient, log: Logger): Promise<"exist" | "not_exist" | "unknown"> => {
+	try {
+		const status = await getIssueStatusFromRedis(jiraHost, issueKey);
+		if (status === "not_exist") {
+			return "not_exist";
+		} else if (status === "exist") {
+			return "exist";
+		}
+		await jiraClient.issues.get(issueKey);
+		await saveIssueStatusToRedis(jiraHost, issueKey, "exist");
+		return "exist";
+	} catch (e) {
+		if (e instanceof JiraClientError) {
+			if (e.status === 404) {
+				//404, issue not exists
+				await saveIssueStatusToRedis(jiraHost, issueKey, "not_exist");
+				return "not_exist";
+			}
+		}
+		//some unknow error,
+		//return true to continue processing the msg for the safe side
+		log.warn({ err: e }, "Found other errors when fetching issue status");
+		return "unknown";
 	}
-	//all issue keys within this commits NOT exist on jira, so should skip processing the msg itself.
-	return false;
 };

src/util/jira-issue-check-redis-util.test.ts

@@ -0,0 +1,29 @@
+import { saveIssueStatusToRedis, getIssueStatusFromRedis } from "./jira-issue-check-redis-util";
+import { numberFlag, NumberFlags } from "config/feature-flags";
+import { when } from "jest-when";
+
+jest.mock("config/feature-flags");
+
+describe("Redis for jira issue status", () => {
+	const TIMEOUT = 10_000;
+	let issueKey: string;
+	beforeEach(async () => {
+		when(numberFlag).calledWith(NumberFlags.SKIP_PROCESS_QUEUE_IF_ISSUE_NOT_FOUND_TIMEOUT, expect.anything(), expect.anything())
+			.mockResolvedValue(TIMEOUT);
+		issueKey = "ABC-" + Math.floor(Math.random() * 10000);
+	});
+	it("should save and successfully retried last status (exist)", async () => {
+		await saveIssueStatusToRedis(jiraHost, issueKey, "exist");
+		const status = await getIssueStatusFromRedis(jiraHost, issueKey);
+		expect(status).toEqual("exist");
+	});
+	it("should save and successfully retried last status (not-exists)", async () => {
+		await saveIssueStatusToRedis(jiraHost, issueKey, "not_exist");
+		const status = await getIssueStatusFromRedis(jiraHost, issueKey);
+		expect(status).toEqual("not_exist");
+	});
+	it("should return null on not found status", async () => {
+		const status = await getIssueStatusFromRedis(jiraHost, issueKey);
+		expect(status).toBeNull();
+	});
+});

src/util/jira-issue-check-redis-util.ts

@@ -1,5 +1,6 @@
 import IORedis  from "ioredis";
 import { getRedisInfo } from "config/redis-info";
+import { numberFlag, NumberFlags } from "config/feature-flags";
 
 //five seconds
 const REDIS_CLEANUP_TIMEOUT = 5 * 1000;
@@ -12,7 +13,8 @@ export const saveIssueStatusToRedis = async (
 	status: "exist" | "not_exist"
 ) => {
 	const key = getKey(jiraHost, issueKey);
-	await redis.set(key, status, "px", REDIS_CLEANUP_TIMEOUT);
+	const timeout = await numberFlag(NumberFlags.SKIP_PROCESS_QUEUE_IF_ISSUE_NOT_FOUND_TIMEOUT, REDIS_CLEANUP_TIMEOUT, jiraHost);
+	await redis.set(key, status, "px", timeout);
 };
 
 export const getIssueStatusFromRedis = async (