Skip to content

Commit d799a7a

Browse files
davidfialadavidfiala
committed
unify server and client keepalive matching comments and discussion on first round of review from #2760 
1 parent 334f0dc commit d799a7a

File tree

2 files changed

+197
-192
lines changed

2 files changed

+197
-192
lines changed

‎packages/grpc-js/src/server.ts

Lines changed: 148 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,14 @@ export class Server {
435435
);
436436
}
437437

438+
private keepaliveTrace(text: string): void {
439+
logging.trace(
440+
LogVerbosity.DEBUG,
441+
'keepalive',
442+
'(' + this.channelzRef.id + ') ' + text
443+
);
444+
}
445+
438446
addProtoService(): never {
439447
throw new Error('Not implemented. Use addService() instead');
440448
}
@@ -1376,7 +1384,8 @@ export class Server {
13761384

13771385
let connectionAgeTimer: NodeJS.Timeout | null = null;
13781386
let connectionAgeGraceTimer: NodeJS.Timeout | null = null;
1379-
let keepaliveInterval: NodeJS.Timeout | null = null;
1387+
let keepaliveTimeout: NodeJS.Timeout | null = null;
1388+
let keepaliveDisabled = false;
13801389
let sessionClosedByServer = false;
13811390

13821391
const idleTimeoutObj = this.enableIdleTimeout(session);
@@ -1419,72 +1428,73 @@ export class Server {
14191428
connectionAgeTimer.unref?.();
14201429
}
14211430

1422-
if (this.keepaliveTimeMs < KEEPALIVE_MAX_TIME_MS) {
1423-
keepaliveInterval = setInterval(() => {
1424-
const keepaliveTimeout = setTimeout(() => {
1425-
if (keepaliveInterval) {
1426-
clearInterval(keepaliveInterval);
1427-
keepaliveInterval = null;
1431+
const clearKeepaliveTimeout = () => {
1432+
if (keepaliveTimeout) {
1433+
clearTimeout(keepaliveTimeout);
1434+
keepaliveTimeout = null;
1435+
}
1436+
};
1437+
1438+
const canSendPing = () => {
1439+
return (
1440+
!keepaliveDisabled &&
1441+
this.keepaliveTimeMs < KEEPALIVE_MAX_TIME_MS &&
1442+
this.keepaliveTimeMs > 0
1443+
);
1444+
};
1445+
1446+
const maybeStartKeepalivePingTimer = () => {
1447+
if (!canSendPing()) {
1448+
return;
1449+
}
1450+
this.keepaliveTrace(
1451+
'Starting keepalive timer for ' + this.keepaliveTimeMs + 'ms'
1452+
);
1453+
keepaliveTimeout = setTimeout(() => {
1454+
clearKeepaliveTimeout();
1455+
sendPing();
1456+
}, this.keepaliveTimeMs);
1457+
keepaliveTimeout.unref?.();
1458+
};
1459+
1460+
const sendPing = () => {
1461+
if (!canSendPing()) {
1462+
return;
1463+
}
1464+
this.keepaliveTrace(
1465+
'Sending ping with timeout ' + this.keepaliveTimeoutMs + 'ms'
1466+
);
1467+
const pingSentSuccessfully = session.ping(
1468+
(err: Error | null, duration: number, payload: Buffer) => {
1469+
clearKeepaliveTimeout();
1470+
if (err) {
1471+
this.keepaliveTrace('Ping failed with error: ' + err.message);
14281472
sessionClosedByServer = true;
1429-
this.trace('Connection dropped by keepalive timeout');
14301473
session.close();
1474+
} else {
1475+
this.keepaliveTrace('Received ping response');
1476+
maybeStartKeepalivePingTimer();
14311477
}
1432-
}, this.keepaliveTimeoutMs);
1433-
keepaliveTimeout.unref?.();
1434-
1435-
try {
1436-
if (
1437-
!session.ping(
1438-
(err: Error | null, duration: number, payload: Buffer) => {
1439-
clearTimeout(keepaliveTimeout);
1440-
if (err) {
1441-
if (keepaliveInterval) {
1442-
clearInterval(keepaliveInterval);
1443-
keepaliveInterval = null;
1444-
}
1445-
sessionClosedByServer = true;
1446-
this.trace(
1447-
'Connection dropped due to error with ping frame ' +
1448-
err.message +
1449-
' return in ' +
1450-
duration
1451-
);
1452-
session.close();
1453-
}
1454-
}
1455-
)
1456-
) {
1457-
throw new Error('Server keepalive ping send failed');
1458-
}
1459-
} catch (e) {
1460-
// The ping can't be sent because the session is already closed, max outstanding pings reached, etc
1461-
clearTimeout(keepaliveTimeout);
1462-
if (keepaliveInterval) {
1463-
clearInterval(keepaliveInterval);
1464-
keepaliveInterval = null;
1465-
}
1466-
this.trace(
1467-
'Connection dropped due to error sending ping frame ' +
1468-
(e instanceof Error ? e.message : 'unknown error')
1469-
);
1470-
session.destroy();
14711478
}
1472-
}, this.keepaliveTimeMs);
1473-
keepaliveInterval.unref?.();
1474-
}
1479+
);
14751480

1476-
session.once('goaway', (errorCode, lastStreamID, opaqueData) => {
1477-
if (errorCode === http2.constants.NGHTTP2_ENHANCE_YOUR_CALM) {
1478-
this.trace('Connection dropped by client due to ENHANCE_YOUR_CALM');
1479-
} else {
1480-
this.trace(
1481-
'Connection dropped by client via GOAWAY with error code ' +
1482-
errorCode
1483-
);
1481+
if (!pingSentSuccessfully) {
1482+
this.keepaliveTrace('Ping failed to send');
1483+
sessionClosedByServer = true;
1484+
session.close();
1485+
return;
14841486
}
1485-
sessionClosedByServer = true;
1486-
session.destroy();
1487-
});
1487+
1488+
keepaliveTimeout = setTimeout(() => {
1489+
clearKeepaliveTimeout();
1490+
this.keepaliveTrace('Ping timeout passed without response');
1491+
sessionClosedByServer = true;
1492+
session.close();
1493+
}, this.keepaliveTimeoutMs);
1494+
keepaliveTimeout.unref?.();
1495+
};
1496+
1497+
maybeStartKeepalivePingTimer();
14881498

14891499
session.on('close', () => {
14901500
if (!sessionClosedByServer) {
@@ -1501,10 +1511,8 @@ export class Server {
15011511
clearTimeout(connectionAgeGraceTimer);
15021512
}
15031513

1504-
if (keepaliveInterval) {
1505-
clearInterval(keepaliveInterval);
1506-
keepaliveInterval = null;
1507-
}
1514+
keepaliveDisabled = true;
1515+
clearKeepaliveTimeout();
15081516

15091517
if (idleTimeoutObj !== null) {
15101518
clearTimeout(idleTimeoutObj.timeout);
@@ -1549,7 +1557,8 @@ export class Server {
15491557

15501558
let connectionAgeTimer: NodeJS.Timeout | null = null;
15511559
let connectionAgeGraceTimer: NodeJS.Timeout | null = null;
1552-
let keepaliveInterval: NodeJS.Timeout | null = null;
1560+
let keepaliveTimeout: NodeJS.Timeout | null = null;
1561+
let keepaliveDisabled = false;
15531562
let sessionClosedByServer = false;
15541563

15551564
const idleTimeoutObj = this.enableIdleTimeout(session);
@@ -1591,85 +1600,90 @@ export class Server {
15911600
connectionAgeTimer.unref?.();
15921601
}
15931602

1594-
if (this.keepaliveTimeMs < KEEPALIVE_MAX_TIME_MS) {
1595-
keepaliveInterval = setInterval(() => {
1596-
const keepaliveTimeout = setTimeout(() => {
1597-
if (keepaliveInterval) {
1598-
clearInterval(keepaliveInterval);
1599-
keepaliveInterval = null;
1600-
sessionClosedByServer = true;
1603+
const clearKeepaliveTimeout = () => {
1604+
if (keepaliveTimeout) {
1605+
clearTimeout(keepaliveTimeout);
1606+
keepaliveTimeout = null;
1607+
}
1608+
};
1609+
1610+
const canSendPing = () => {
1611+
return (
1612+
!keepaliveDisabled &&
1613+
this.keepaliveTimeMs < KEEPALIVE_MAX_TIME_MS &&
1614+
this.keepaliveTimeMs > 0
1615+
);
1616+
};
1617+
1618+
const maybeStartKeepalivePingTimer = () => {
1619+
if (!canSendPing()) {
1620+
return;
1621+
}
1622+
this.keepaliveTrace(
1623+
'Starting keepalive timer for ' + this.keepaliveTimeMs + 'ms'
1624+
);
1625+
keepaliveTimeout = setTimeout(() => {
1626+
clearKeepaliveTimeout();
1627+
sendPing();
1628+
}, this.keepaliveTimeMs);
1629+
keepaliveTimeout.unref?.();
1630+
};
1631+
1632+
const sendPing = () => {
1633+
if (!canSendPing()) {
1634+
return;
1635+
}
1636+
this.keepaliveTrace(
1637+
'Sending ping with timeout ' + this.keepaliveTimeoutMs + 'ms'
1638+
);
1639+
const pingSentSuccessfully = session.ping(
1640+
(err: Error | null, duration: number, payload: Buffer) => {
1641+
clearKeepaliveTimeout();
1642+
if (err) {
1643+
this.keepaliveTrace('Ping failed with error: ' + err.message);
16011644
this.channelzTrace.addTrace(
16021645
'CT_INFO',
1603-
'Connection dropped by keepalive timeout from ' + clientAddress
1646+
'Connection dropped due to error of a ping frame ' +
1647+
err.message +
1648+
' return in ' +
1649+
duration
16041650
);
1651+
sessionClosedByServer = true;
16051652
session.close();
1653+
} else {
1654+
this.keepaliveTrace('Received ping response');
1655+
maybeStartKeepalivePingTimer();
16061656
}
1607-
}, this.keepaliveTimeoutMs);
1608-
keepaliveTimeout.unref?.();
1609-
1610-
try {
1611-
if (
1612-
!session.ping(
1613-
(err: Error | null, duration: number, payload: Buffer) => {
1614-
clearTimeout(keepaliveTimeout);
1615-
if (err) {
1616-
if (keepaliveInterval) {
1617-
clearInterval(keepaliveInterval);
1618-
keepaliveInterval = null;
1619-
}
1620-
sessionClosedByServer = true;
1621-
this.channelzTrace.addTrace(
1622-
'CT_INFO',
1623-
'Connection dropped due to error with ping frame ' +
1624-
err.message +
1625-
' return in ' +
1626-
duration
1627-
);
1628-
session.close();
1629-
}
1630-
}
1631-
)
1632-
) {
1633-
throw new Error('Server keepalive ping send failed');
1634-
}
1635-
channelzSessionInfo.keepAlivesSent += 1;
1636-
} catch (e) {
1637-
// The ping can't be sent because the session is already closed, max outstanding pings reached, etc
1638-
clearTimeout(keepaliveTimeout);
1639-
if (keepaliveInterval) {
1640-
clearInterval(keepaliveInterval);
1641-
keepaliveInterval = null;
1642-
}
1643-
this.channelzTrace.addTrace(
1644-
'CT_INFO',
1645-
'Connection dropped due to error sending ping frame ' +
1646-
(e instanceof Error ? e.message : 'unknown error')
1647-
);
1648-
session.destroy();
16491657
}
1650-
}, this.keepaliveTimeMs);
1651-
keepaliveInterval.unref?.();
1652-
}
1658+
);
16531659

1654-
session.once('goaway', (errorCode, lastStreamID, opaqueData) => {
1655-
if (errorCode === http2.constants.NGHTTP2_ENHANCE_YOUR_CALM) {
1660+
if (!pingSentSuccessfully) {
1661+
this.keepaliveTrace('Ping failed to send');
16561662
this.channelzTrace.addTrace(
16571663
'CT_INFO',
1658-
'Connection dropped by client due GOAWAY of ENHANCE_YOUR_CALM from ' +
1659-
clientAddress
1664+
'Connection dropped due failure to send ping frame'
16601665
);
1661-
} else {
1666+
sessionClosedByServer = true;
1667+
session.close();
1668+
return;
1669+
}
1670+
1671+
channelzSessionInfo.keepAlivesSent += 1;
1672+
1673+
keepaliveTimeout = setTimeout(() => {
1674+
clearKeepaliveTimeout();
1675+
this.keepaliveTrace('Ping timeout passed without response');
16621676
this.channelzTrace.addTrace(
16631677
'CT_INFO',
1664-
'Connection dropped by client via GOAWAY with error code ' +
1665-
errorCode +
1666-
' from ' +
1667-
clientAddress
1678+
'Connection dropped by keepalive timeout from ' + clientAddress
16681679
);
1669-
}
1670-
sessionClosedByServer = true;
1671-
session.destroy();
1672-
});
1680+
sessionClosedByServer = true;
1681+
session.close();
1682+
}, this.keepaliveTimeoutMs);
1683+
keepaliveTimeout.unref?.();
1684+
};
1685+
1686+
maybeStartKeepalivePingTimer();
16731687

16741688
session.on('close', () => {
16751689
if (!sessionClosedByServer) {
@@ -1690,10 +1704,8 @@ export class Server {
16901704
clearTimeout(connectionAgeGraceTimer);
16911705
}
16921706

1693-
if (keepaliveInterval) {
1694-
clearInterval(keepaliveInterval);
1695-
keepaliveInterval = null;
1696-
}
1707+
keepaliveDisabled = true;
1708+
clearKeepaliveTimeout();
16971709

16981710
if (idleTimeoutObj !== null) {
16991711
clearTimeout(idleTimeoutObj.timeout);

0 commit comments

Comments
 (0)