Skip to content

Commit

Permalink
fix: handle errors fetching actions and vouchers from the DB
Browse files Browse the repository at this point in the history
This fix adds error handling to the two timers that periodically query
the database: one which gets actions from the queue, and other that gets pending
vouchers. In tests, the DB tables are dropped, so the timers would through
errors and cause tests to fail. In production whatever errors cause
a DB fetch to fail could be recoverable so the timer will keep retrying
rather than bubbling up an exception, so hopefully that is okay.

We also change CI to only build using Ubuntu 22.04
(the node 18 version has dependency issues on Ubuntu 20.04).

Additionally we make CI tests and formatting check happen on all PRs, not
only the ones to main.
  • Loading branch information
pcarranzav committed Jul 31, 2023
1 parent 109a661 commit 6849b0f
Show file tree
Hide file tree
Showing 14 changed files with 43 additions and 24 deletions.
3 changes: 1 addition & 2 deletions .github/workflows/check-formatting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ name: "Check Formatting"
on:
push:
branches: [main]
pull_request:
branches: [main]
pull_request: {}

jobs:
check:
Expand Down
9 changes: 2 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,15 @@ name: CI
on:
push:
branches: [main]
pull_request:
branches: [main]
pull_request: {}

jobs:
build:
strategy:
matrix:
node-version: [16, 17, 18]
system:
- os: ubuntu-20.04
include:
- node_version: 18
system:
os: ubuntu-22.04
- os: ubuntu-22.04
runs-on: ${{ matrix.system.os }}
services:
postgres:
Expand Down
2 changes: 1 addition & 1 deletion packages/indexer-agent/src/__tests__/indexer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ const setup = async () => {
sequelize = await connectDatabase(__DATABASE__)
models = defineIndexerManagementModels(sequelize)
queryFeeModels = defineQueryFeeModels(sequelize)
await sequelize.sync({ force: true })
sequelize = await sequelize.sync({ force: true })

const indexNodeIDs = ['node_1']

Expand Down
2 changes: 1 addition & 1 deletion packages/indexer-cli/src/__tests__/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export const setup = async () => {
// Clearing the registry prevents duplicate metric registration in the default registry.
metrics.registry.clear()

await sequelize.sync({ force: true })
sequelize = await sequelize.sync({ force: true })

const statusEndpoint = 'http://localhost:8030/graphql'
const indexNodeIDs = ['node_1']
Expand Down
2 changes: 1 addition & 1 deletion packages/indexer-common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"compile": "tsc",
"prepare": "yarn format && yarn lint && yarn compile",
"test": "LOG_LEVEL=info jest --colors --verbose --runInBand --detectOpenHandles",
"test:ci": "LOG_LEVEL=error jest --verbose --runInBand --ci",
"test:ci": "LOG_LEVEL=info jest --verbose --maxWorkers=1 --ci",
"test:debug": "LOG_LEVEL=debug jest --runInBand --detectOpenHandles --verbose",
"test:watch": "jest --runInBand --detectOpenHandles --watch --passWithNoTests --verbose",
"clean": "rm -rf ./node_modules ./dist ./tsconfig.tsbuildinfo"
Expand Down
8 changes: 7 additions & 1 deletion packages/indexer-common/src/allocations/query-fees.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,13 @@ export class AllocationReceiptCollector implements ReceiptCollector {

private startVoucherProcessing() {
timer(30_000).pipe(async () => {
const pendingVouchers = await this.pendingVouchers() // Ordered by value
let pendingVouchers: Voucher[] = []
try {
pendingVouchers = await this.pendingVouchers() // Ordered by value
} catch (err) {
this.logger.warn(`Failed to query pending vouchers`, { err })
return
}

const logger = this.logger.child({})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const setup = async () => {
sequelize = await connectDatabase(__DATABASE__)
managementModels = defineIndexerManagementModels(sequelize)
queryFeeModels = defineQueryFeeModels(sequelize)
sequelize = await sequelize.sync({ force: true })

const graphNode = new GraphNode(
logger,
Expand Down Expand Up @@ -99,7 +100,9 @@ const teardownAll = async () => {

describe('Allocation Manager', () => {
beforeAll(setup)
beforeEach(setupEach)
beforeEach(() => {
return setupEach()
})
afterEach(teardownEach)
afterAll(teardownAll)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
IndexingDecisionBasis,
IndexingRuleAttributes,
} from '../models'
import { specification as spec } from '../../index'
import { defineQueryFeeModels, specification as spec } from '../../index'
import { networkIsL1, networkIsL2 } from '../types'
import { fetchIndexingRules, upsertIndexingRule } from '../rules'
import { SubgraphIdentifierType } from '../../subgraphs'
Expand Down Expand Up @@ -58,7 +58,8 @@ const setupModels = async () => {
// Spin up db
sequelize = await connectDatabase(__DATABASE__)
models = defineIndexerManagementModels(sequelize)
await sequelize.sync({ force: true })
defineQueryFeeModels(sequelize)
sequelize = await sequelize.sync({ force: true })
}

const setupMonitor = async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { defineIndexerManagementModels, IndexerManagementModels } from '../../mo
import { CombinedError } from '@urql/core'
import { GraphQLError } from 'graphql'
import { createTestManagementClient } from '../util'
import { defineQueryFeeModels } from '../../../query-fees/models'

// Make global Jest variable available
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -83,6 +84,7 @@ const setupAll = async () => {
// Spin up db
sequelize = await connectDatabase(__DATABASE__)
models = defineIndexerManagementModels(sequelize)
defineQueryFeeModels(sequelize)
sequelize = await sequelize.sync({ force: true })
logger = createLogger({
name: 'Indexer API Client',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ import {
IndexingDecisionBasis,
INDEXING_RULE_GLOBAL,
} from '../../models'
import { SubgraphIdentifierType } from '@graphprotocol/indexer-common'
import {
SubgraphIdentifierType,
defineQueryFeeModels,
} from '@graphprotocol/indexer-common'

import { createTestManagementClient, defaults } from '../util'

Expand Down Expand Up @@ -113,6 +116,7 @@ const metrics = createMetrics()
const setupAll = async () => {
sequelize = await connectDatabase(__DATABASE__)
models = defineIndexerManagementModels(sequelize)
defineQueryFeeModels(sequelize)
await sequelize.sync({ force: true })

logger = createLogger({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
POIDisputeAttributes,
} from '../../models'
import { createTestManagementClient } from '../util'
import { defineQueryFeeModels } from '../../../query-fees/models'

// Make global Jest variable available
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -187,6 +188,8 @@ const setupAll = async () => {
// Spin up db
sequelize = await connectDatabase(__DATABASE__)
models = defineIndexerManagementModels(sequelize)
defineQueryFeeModels(sequelize)
sequelize = await sequelize.sync({ force: true })
logger = createLogger({
name: 'Indexer API Client',
async: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ export const createTestManagementClient = async (
metrics.registry.clear()

// Spin up db
const sequelize = await connectDatabase(databaseOptions)
let sequelize = await connectDatabase(databaseOptions)
const queryFeeModels = defineQueryFeeModels(sequelize)
const managementModels = defineIndexerManagementModels(sequelize)
await sequelize.sync({ force: true })
sequelize = await sequelize.sync({ force: true })
const statusEndpoint = 'http://localhost:8030/graphql'
const graphNode = new GraphNode(
logger,
Expand Down
14 changes: 10 additions & 4 deletions packages/indexer-common/src/indexer-management/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,16 @@ export class ActionManager {
const approvedActions: Eventual<Action[]> = timer(30_000).tryMap(
async () => {
logger.trace('Fetching approved actions')
const actions = await ActionManager.fetchActions(this.models, {
status: ActionStatus.APPROVED,
})
logger.trace(`Fetched ${actions.length} approved actions`)
let actions: Action[] = []
try {
actions = await ActionManager.fetchActions(this.models, {
status: ActionStatus.APPROVED,
})
logger.trace(`Fetched ${actions.length} approved actions`)
} catch (err) {
logger.warn('Failed to fetch approved actions from queue', { err })
}

return actions
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const setup = async () => {
models = defineIndexerManagementModels(sequelize)
address = '0x90f8bf6a479f320ead074411a4b0e7944ea8c9c1'
contracts = await connectContracts(getTestProvider('goerli'), 5)
await sequelize.sync({ force: true })
sequelize = await sequelize.sync({ force: true })
const statusEndpoint = 'http://localhost:8030/graphql'
indexingStatusResolver = new IndexingStatusResolver({
logger: logger,
Expand Down

0 comments on commit 6849b0f

Please sign in to comment.