Skip to content

Commit

Permalink
Merge pull request #3450 from magda-io/issue/3449
Browse files Browse the repository at this point in the history
Issue/3449 Gateway ckanRedirection module performance improvement
  • Loading branch information
t83714 authored Feb 28, 2023
2 parents 4baabb5 + 8c395ce commit 0b90334
Show file tree
Hide file tree
Showing 17 changed files with 405 additions and 33 deletions.
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
## v2.2.3

- Fixed an issue that zendesk integration feedback form might not always be opened via footer link
- #3449: Gateway ckanRedirection module performance improvement
- related to #3448: make web server module to use read only registry node for site map generation

## v2.2.2

Expand Down
6 changes: 4 additions & 2 deletions deploy/helm/internal-charts/gateway/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ Kubernetes: `>= 1.14.0-0`
| autoscaler.maxReplicas | int | `3` | |
| autoscaler.minReplicas | int | `1` | |
| autoscaler.targetCPUUtilizationPercentage | int | `80` | |
| ckanRedirectionDomain | string | `nil` | CKAN redirection target CKAN instance domain. See `enableCkanRedirection` for more details. |
| ckanRedirectionPath | string | `nil` | CKAN redirection target CKAN instance path. See `enableCkanRedirection` for more details. |
| ckanRedirectionDomain | string | `nil` | CKAN redirection target CKAN instance domain (e.g. `data.gov.au`). See `enableCkanRedirection` for more details. |
| ckanRedirectionPath | string | `nil` | CKAN redirection target CKAN instance path (e.g. `/data`). See `enableCkanRedirection` for more details. |
| cookie | object | default value see `Description` | Session cookie settings. <br/> More info: https://github.com/expressjs/session#cookie <br/> Supported options are:<br/> <ul> <li>`expires`: A fix cookie expire date. The expires option should not be set directly; instead only use the maxAge option.</li> <li>`httpOnly`: Default: true.</li> <li>`maxAge`: Default: 7 * 60 * 60 * 1000 (7 hours)</li> <li>`path`: Default: '/'.</li> <li>`sameSite`: Default: lax </li> <li>`secure`: Default: "auto" </li> </ul> |
| cors.exposedHeaders[0] | string | `"Content-Range"` | |
| cors.exposedHeaders[1] | string | `"X-Content-Range"` | |
Expand Down Expand Up @@ -51,6 +51,8 @@ Kubernetes: `>= 1.14.0-0`
| helmet.frameguard | bool | `false` | |
| image.name | string | `"magda-gateway"` | |
| proxyTimeout | int | nil (120 seconds default value will be used by upstream lib internally) | How long time (in seconds) before upstream service must complete request in order to avoid request timeout error. If not set, the request will time out after 120 seconds. |
| registryQueryCacheMaxKeys | number | `nil` | Specifies a maximum amount of keys that can be stored in the registryQueryCache. By default, it will be set to 500 seconds if leave blank. |
| registryQueryCacheStdTTL | number | `nil` | The standard ttl as number in seconds for every generated cache element in the registryQueryCache. To disable the cache, set this value to `0`. By default, it will be set to 600 seconds if leave blank. |
| resources.limits.cpu | string | `"200m"` | |
| resources.requests.cpu | string | `"50m"` | |
| resources.requests.memory | string | `"40Mi"` | |
Expand Down
6 changes: 6 additions & 0 deletions deploy/helm/internal-charts/gateway/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ spec:
{{- end }}
{{- if .Values.enableCkanRedirection }}
"--enableCkanRedirection", {{ .Values.enableCkanRedirection | quote }},
{{- end }}
{{- if not (kindIs "invalid" .Values.registryQueryCacheStdTTL) }}
"--registryQueryCacheStdTTL", {{ .Values.registryQueryCacheStdTTL | quote }},
{{- end }}
{{- if .Values.registryQueryCacheMaxKeys }}
"--registryQueryCacheMaxKeys", {{ .Values.registryQueryCacheMaxKeys | quote }},
{{- end }}
"--proxyRoutesJson", "/etc/config/routes.json",
"--webProxyRoutesJson", "/etc/config/webRoutes.json",
Expand Down
13 changes: 11 additions & 2 deletions deploy/helm/internal-charts/gateway/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,19 @@ enableWebAccessControl: false
# that is specified by config option `ckanRedirectionDomain` and `ckanRedirectionPath`.
enableCkanRedirection: false

# -- CKAN redirection target CKAN instance domain. See `enableCkanRedirection` for more details.
# -- (number) The standard ttl as number in seconds for every generated cache element in the registryQueryCache.
# To disable the cache, set this value to `0`.
# By default, it will be set to 600 seconds if leave blank.
registryQueryCacheStdTTL:

# -- (number) Specifies a maximum amount of keys that can be stored in the registryQueryCache.
# By default, it will be set to 500 seconds if leave blank.
registryQueryCacheMaxKeys:

# -- CKAN redirection target CKAN instance domain (e.g. `data.gov.au`). See `enableCkanRedirection` for more details.
ckanRedirectionDomain:

# -- CKAN redirection target CKAN instance path. See `enableCkanRedirection` for more details.
# -- CKAN redirection target CKAN instance path (e.g. `/data`). See `enableCkanRedirection` for more details.
ckanRedirectionPath:


Expand Down
2 changes: 1 addition & 1 deletion deploy/helm/internal-charts/web-server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ Kubernetes: `>= 1.14.0-0`
| openInExternalTerriaMapButtonText | string | `nil` | When set, the string here will replace the text of the `Open in National Map` button in Map Preview area. |
| openInExternalTerriaMapTargetUrl | string | `nil` | When set, the `Open in National Map` button in Map Preview area will sent map data to the URL provided and open the map preview there. When not set, UI will by default send to the National Map. |
| previewMapFormatPerference | list | `[{"format":"WMS","urlRegex":"^(?!.*(SceneServer)).*$"},{"format":"ESRI MAPSERVER","urlRegex":"MapServer"},{"format":"WFS","urlRegex":"^(?!.*(SceneServer)).*$"},{"format":"ESRI FEATURESERVER","urlRegex":"FeatureServer"},{"format":"GeoJSON","singleFile":true},{"format":"csv-geo-au","singleFile":true},{"format":"KML","singleFile":true},{"format":"KMZ","singleFile":true}]` | Preview map module format perference list The list includes one or more `format perference item`. When there are more than one data source available, "Preview Map module" will use this perference to determine which data soruce will be used. It will go through the perference list. The first matched format (i.e. find a data source with the format ) will be chosen. A `format perference item` can have the following fields: <ul> <li>format: the format of the preferred data source. compulsory. case insensitive. </li> <li> isDataFile: Optional. Default to `false`. Indicate whether the specified format is a static data file or API. If it's a static file, "Preview Map Module" will attempt to check the target file size and ask user to confirm whether he wants to render the file for large files. The file size threshold is specified by config option `automaticPreviewMaxFileSize`. </li> <li> urlRegex: optional; when exists, it will be used as regex string to double check the data source access url to confirm whether it's indeed the format specified. If not provided or empty, only `format` will be used to determine whether a data source matches the `format perference item`. <li> </ul> |
| registryApiBaseUrlInternal | string | `"http://registry-api/v0"` | |
| registryApiBaseUrlInternal | string | `"http://registry-api-read-only/v0"` | |
| replicas | string | `nil` | no. of replicas required for the deployment. If not set, k8s will assume `1` but allows HPA (autoscaler) alters it. @default 1 |
| resources.limits.cpu | string | `"100m"` | |
| resources.requests.cpu | string | `"10m"` | |
Expand Down
2 changes: 1 addition & 1 deletion deploy/helm/internal-charts/web-server/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ useLocalStyleSheet: false

contentApiBaseUrlInternal: "http://content-api/v0/"

registryApiBaseUrlInternal: "http://registry-api/v0"
registryApiBaseUrlInternal: "http://registry-api-read-only/v0"

adminApiBaseUrl:

Expand Down
1 change: 1 addition & 0 deletions magda-gateway/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"pg": "^8.7.3",
"read-pkg-up": "^3.0.0",
"request": "^2.88.0",
"node-cache": "^5.1.2",
"tsmonad": "^0.7.2",
"urijs": "^1.19.11",
"uuid": "^8.2.0",
Expand Down
14 changes: 12 additions & 2 deletions magda-gateway/src/buildApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import AuthDecisionQueryClient from "magda-typescript-common/src/opa/AuthDecisio
type Route = {
to: string;
auth?: boolean;
methods?: { method: string; target: string }[];
};

type Routes = {
Expand Down Expand Up @@ -74,6 +75,8 @@ export type Config = {
magdaAdminPortalName?: string;
proxyTimeout?: string;
skipAuth?: boolean;
registryQueryCacheMaxKeys: number;
registryQueryCacheStdTTL: number;
};

export default function buildApp(app: express.Application, config: Config) {
Expand Down Expand Up @@ -220,12 +223,19 @@ export default function buildApp(app: express.Application, config: Config) {
"Cannot locate routes.registry for ckan redirection!"
);
} else {
const registryReadOnlyApi = routes.registry?.methods?.find(
(item) => item.method.toLowerCase() === "get"
)?.target;
mainRouter.use(
createCkanRedirectionRouter({
ckanRedirectionDomain: config.ckanRedirectionDomain,
ckanRedirectionPath: config.ckanRedirectionPath,
registryApiBaseUrlInternal: routes.registry.to,
tenantId: 0 // FIXME: Rather than being hard-coded to the default tenant, the CKAN router needs to figure out the correct tenant.
registryApiBaseUrlInternal: registryReadOnlyApi
? registryReadOnlyApi
: routes.registry?.to,
tenantId: 0, // FIXME: Rather than being hard-coded to the default tenant, the CKAN router needs to figure out the correct tenant.,
cacheStdTTL: config.registryQueryCacheStdTTL,
cacheMaxKeys: config.registryQueryCacheMaxKeys
})
);
}
Expand Down
46 changes: 42 additions & 4 deletions magda-gateway/src/createCkanRedirectionRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@ import _ from "lodash";
import Registry from "magda-typescript-common/src/registry/RegistryClient";
import unionToThrowable from "magda-typescript-common/src/util/unionToThrowable";
import { escapeRegExp } from "lodash";
import NodeCache from "node-cache";

export type CkanRedirectionRouterOptions = {
ckanRedirectionDomain: string;
ckanRedirectionPath: string;
registryApiBaseUrlInternal: string;
tenantId: number;
cacheStdTTL: number;
cacheMaxKeys: number;
};

export type genericUrlRedirectConfig =
Expand Down Expand Up @@ -76,8 +79,17 @@ export default function buildCkanRedirectionRouter({
ckanRedirectionDomain,
ckanRedirectionPath,
registryApiBaseUrlInternal,
tenantId
tenantId,
cacheStdTTL,
cacheMaxKeys
}: CkanRedirectionRouterOptions): express.Router {
const registryQueryCache =
cacheMaxKeys === 0 // turn off the cache when cacheMaxKeys==0
? null
: new NodeCache({
stdTTL: cacheStdTTL,
maxKeys: cacheMaxKeys
});
const router = express.Router();
const registry = new Registry({
baseUrl: registryApiBaseUrlInternal,
Expand Down Expand Up @@ -162,11 +174,31 @@ export default function buildCkanRedirectionRouter({
);
});

function getQueryRegistryRecordApiCacheKey(
aspectQuery: string[],
aspect: string[],
limit: number
) {
return JSON.stringify({
aspectQuery,
aspect,
limit
});
}

async function queryRegistryRecordApi(
aspectQuery: string[],
aspect: string[],
limit: number = 1
limit: number = undefined
) {
const cacheKey = getQueryRegistryRecordApiCacheKey(
aspectQuery,
aspect,
limit
);
if (registryQueryCache?.has(cacheKey)) {
return registryQueryCache.get(cacheKey);
}
const queryParameters: any = {
limit
};
Expand All @@ -188,6 +220,12 @@ export default function buildCkanRedirectionRouter({
)
);

try {
registryQueryCache?.set(cacheKey, records);
} catch (e) {
console.log("Failed to save registryQueryCache: " + e);
}

return records;
}

Expand All @@ -196,7 +234,7 @@ export default function buildCkanRedirectionRouter({
aspectName: string,
retrieveAspectContent: boolean = true,
retrieveAspects: string[] = [],
limit: number = 1
limit: number = undefined
): Promise<any[]> {
const query = `${aspectName}.${
uuidRegEx.test(ckanIdOrName) ? "id" : "name"
Expand Down Expand Up @@ -232,7 +270,7 @@ export default function buildCkanRedirectionRouter({
ckanIdOrName: string,
aspectName: string
) {
const records = await queryCkanAspect(ckanIdOrName, aspectName, false);
const records = await queryCkanAspect(ckanIdOrName, aspectName);
if (!records || !records.length || !records[0]["id"]) {
return null;
} else {
Expand Down
11 changes: 10 additions & 1 deletion magda-gateway/src/defaultConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,16 @@ export default {
},
registry: {
to: "http://localhost:6101/v0",
auth: true
auth: true,
methods: [
{ method: "get", target: "http://localhost:6101/v0" },
{ method: "head", target: "http://localhost:6101/v0" },
{ method: "options", target: "http://localhost:6101/v0" },
{ method: "post", target: "http://localhost:6101/v0" },
{ method: "put", target: "http://localhost:6101/v0" },
{ method: "patch", target: "http://localhost:6101/v0" },
{ method: "delete", target: "http://localhost:6101/v0" }
]
},
"registry-read-only": {
to: "http://localhost:6101/v0",
Expand Down
12 changes: 12 additions & 0 deletions magda-gateway/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,18 @@ const argv = addJwtSecretFromEnvVar(
type: "string",
default: ""
})
.option("registryQueryCacheStdTTL", {
describe:
"the standard ttl as number in seconds for every generated cache element in the registryQueryCache.",
type: "number",
default: 600
})
.option("registryQueryCacheMaxKeys", {
describe:
"specifies a maximum amount of keys that can be stored in the registryQueryCache. To disable the cache, set this value to `0`.",
type: "number",
default: 500
})
.option("enableWebAccessControl", {
describe:
"Whether users are required to enter a username & password to access the magda web interface",
Expand Down
Loading

0 comments on commit 0b90334

Please sign in to comment.