Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: dns caching manager #1294

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"formik": "^2.4.6",
"gravatar-url": "3.1.0",
"lodash": "4.17.21",
"lru-cache": "^11.0.2",
"mime": "3",
"next": "^14.2.4",
"node-cache": "5.1.2",
Expand Down
9 changes: 9 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

52 changes: 52 additions & 0 deletions server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import imageproxy from '@server/routes/imageproxy';
import { appDataPermissions } from '@server/utils/appDataVolume';
import { getAppVersion } from '@server/utils/appVersion';
import createCustomProxyAgent from '@server/utils/customProxyAgent';
import { dnsCache } from '@server/utils/dnsCacheManager';
import restartFlag from '@server/utils/restartFlag';
import { getClientIp } from '@supercharge/request-ip';
import { TypeormStore } from 'connect-typeorm/out';
Expand Down Expand Up @@ -86,6 +87,57 @@ app
);
}

// Add DNS caching
dns.lookup = (() => {
const wrappedLookup = function (
hostname: string,
options:
| number
| dns.LookupOneOptions
| dns.LookupOptions
| dns.LookupAllOptions,
callback: (
err: NodeJS.ErrnoException | null,
address: string | dns.LookupAddress[] | undefined,
family?: number
) => void
): void {
if (typeof options === 'function') {
callback = options;
options = {};
}

dnsCache
.lookup(hostname)
.then((result) => {
if ((options as dns.LookupOptions).all) {
callback(null, [
{ address: result.address, family: result.family },
]);
} else {
callback(null, result.address, result.family);
}
})
.catch((error) => {
callback(error, undefined, undefined);
});
} as typeof dns.lookup;

(wrappedLookup as any).__promisify__ = async function (
hostname: string,
options?: dns.LookupAllOptions | dns.LookupOneOptions
): Promise<dns.LookupAddress[] | { address: string; family: number }> {
const result = await dnsCache.lookup(hostname);

if (options && 'all' in options && options.all === true) {
return [{ address: result.address, family: result.family }];
}
return { address: result.address, family: result.family };
};

return wrappedLookup;
})();

Comment on lines +90 to +140
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't you implement this inside the dnsCacheManagerItself? It floods the server a bit.

Copy link
Owner Author

@fallenbagel fallenbagel Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flood is coming from the debug logging right? Thats to be removed. I have it added rn to sort of test it. Or if not that what do you mean?

Copy link
Collaborator

@gauthier-th gauthier-th Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No not that, i meant you could create a function inside server/utils/dnsCacheManager.ts that contains all this, because all this code to overwrite the lookup function takes a lot of space inside the startup function of the server.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. And then call it here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

// Register HTTP proxy
if (settings.main.proxy.enabled) {
await createCustomProxyAgent(settings.main.proxy);
Expand Down
16 changes: 16 additions & 0 deletions server/routes/settings/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import discoverSettingRoutes from '@server/routes/settings/discover';
import { ApiError } from '@server/types/error';
import { appDataPath } from '@server/utils/appDataVolume';
import { getAppVersion } from '@server/utils/appVersion';
import { dnsCache } from '@server/utils/dnsCacheManager';
import { getHostname } from '@server/utils/getHostname';
import { Router } from 'express';
import rateLimit from 'express-rate-limit';
Expand Down Expand Up @@ -59,8 +60,23 @@ const filteredMainSettings = (
return main;
};

settingsRoutes.get('/dnsCache', async (req, res) => {
const stats = dnsCache.getStats();
const entries = dnsCache.getCacheEntries();

res.json({
stats,
entries,
});
});

settingsRoutes.get('/main', (req, res, next) => {
const settings = getSettings();
const stats = dnsCache.getStats();
const entries = dnsCache.getCacheEntries();

console.log(entries);
console.log(stats);

if (!req.user) {
return next({ status: 400, message: 'User missing from request.' });
Expand Down
Loading
Loading