This blocks the secondary nodes from accepting any other traffic other than health checks while the current dbVersion is less than the latest migration specifying wrap_in_transaction_and_block_requests.
Depends on D13212
Details
Details
I created two migrations and a new keyserver docker image.
Both migrations included await sleep. The first migration specified wrap_in_transaction_and_block_requests and the second migration specified run_simultaneously_with_requests.
While the first migration was running (and the primary node was unavailable due to running the migration), only health checks were available on the load balancer, meaning that the secondary nodes were only accepting health check traffic.
I also console logged to ensure the loop was running. On the second migration however, all endpoints were available for secondary nodes.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
In this diff, we don't use two express servers. Express servers start with only health checks. Endpoints are dynamically added after we confirm the migration has occurred.
In D13172, we have an express server initialized in the master process. This express server didn't exist before. The situation was that we need the health check to be available during the migration and so needed to be initialized in the master process. However, this express server conflicts with later express servers in non-master processes that listen on the same port.
For example, if there's a hanging request on the master express server, the actual keyserver endpoints would never become available until all requests are resolved. This unfortunately doesn't seem to be guaranteed unless we use something like stoppable to forcibly shutdown the master express server so non-master servers can listen on the port.
This diff doesn't require the health check to exist prior to any non-master process code and so we can simply start with a health check and add endpoints later with a single express server.
| keyserver/src/keyserver.js | ||
|---|---|---|
| 213 ↗ | (On Diff #43960) | Why'd you opt to sleep first? Doesn't this slow down all secondary node startup by 5s? |
| keyserver/src/keyserver.js | ||
|---|---|---|
| 213 ↗ | (On Diff #43960) | Even if we slept after we set dbVersion, wouldn't it still slow down secondary node startup by 5s? Was wondering if we needed to do another dbVersion < latestWrapInTransactionAndBlockRequestsVersion check inside the do block, but thought against it. Code clarity wise, it likely makes more sense to flip them, so I'll do that right now. |
| keyserver/src/keyserver.js | ||
|---|---|---|
| 213 ↗ | (On Diff #43960) | |