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.

Sign up for GitHub

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

[IND-481] Remove market cache. #818

Merged
merged 3 commits into from
Nov 29, 2023
Merged

[IND-481] Remove market cache. #818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
24f2464
[IND-481] Remove market cache.
lcwik Nov 29, 2023
44b977d
Fix lint
lcwik Nov 29, 2023
d8e593a
fix lint
lcwik Nov 29, 2023
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
31 changes: 0 additions & 31 deletions indexer/packages/postgres/__tests__/loops/market-refresher.test.ts
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion indexer/packages/postgres/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export * as ComplianceTable from './stores/compliance-table';

export * as perpetualMarketRefresher from './loops/perpetual-market-refresher';
export * as assetRefresher from './loops/asset-refresher';
export * as marketRefresher from './loops/market-refresher';
export * as liquidityTierRefresher from './loops/liquidity-tier-refresher';

export * as uuid from './helpers/uuid';
Expand Down
76 changes: 0 additions & 76 deletions indexer/packages/postgres/src/loops/market-refresher.ts
View file
Open in desktop

This file was deleted.

5 changes: 0 additions & 5 deletions indexer/services/ender/__tests__/caches/block-cache.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
OraclePriceTable,
assetRefresher,
dbHelpers,
marketRefresher,
perpetualMarketRefresher,
testConstants,
testMocks,
Expand Down Expand Up @@ -35,7 +34,6 @@ describe('block-cache', () => {
clearCandlesMap();
perpetualMarketRefresher.clear();
assetRefresher.clear();
marketRefresher.clear();
});

afterAll(async () => {
Expand Down Expand Up @@ -80,7 +78,6 @@ describe('block-cache', () => {
expect(getCandlesMap()).not.toEqual({});
expect(perpetualMarketRefresher.getPerpetualMarketsMap()).not.toEqual({});
expect(assetRefresher.getAssetsMap()).not.toEqual({});
Comment on lines 79 to 80
Copy link
Contributor

Choose a reason for hiding this comment

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

The summary indicates that references to marketRefresher have been removed, but the test assertions still check the state of perpetualMarketRefresher.getPerpetualMarketsMap() and assetRefresher.getAssetsMap(). These assertions should be removed if marketRefresher and its methods are no longer used.

expect(marketRefresher.getMarketsMap()).not.toEqual({});
}
});
});
Expand All @@ -92,7 +89,6 @@ describe('block-cache', () => {
expect(getCandlesMap()).toEqual({});
expect(perpetualMarketRefresher.getPerpetualMarketsMap()).toEqual({});
expect(assetRefresher.getAssetsMap()).toEqual({});
Comment on lines 90 to 91
Copy link
Contributor

Choose a reason for hiding this comment

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

The test setup still contains calls to perpetualMarketRefresher.getPerpetualMarketsMap() and assetRefresher.getAssetsMap(), which should be removed according to the summary that states all references to marketRefresher have been removed.

expect(marketRefresher.getMarketsMap()).toEqual({});

await initializeAllCaches();

Expand All @@ -101,7 +97,6 @@ describe('block-cache', () => {
expect(getCandlesMap()).not.toEqual({});
expect(perpetualMarketRefresher.getPerpetualMarketsMap()).not.toEqual({});
expect(assetRefresher.getAssetsMap()).not.toEqual({});
Comment on lines 98 to 99
Copy link
Contributor

Choose a reason for hiding this comment

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

The test assertions still check the state of perpetualMarketRefresher.getPerpetualMarketsMap() and assetRefresher.getAssetsMap(). If these are related to the marketRefresher that was removed, these assertions should also be removed.

expect(marketRefresher.getMarketsMap()).not.toEqual({});
});
});
});
2 changes: 0 additions & 2 deletions indexer/services/ender/__tests__/handlers/asset-handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
MarketTable,
Ordering,
testConstants,
marketRefresher,
BlockTable,
TendermintEventTable,
assetRefresher,
Expand Down Expand Up @@ -112,7 +111,6 @@ describe('assetHandler', () => {

it('creates new asset', async () => {
await MarketTable.create(testConstants.defaultMarket);
await marketRefresher.updateMarkets();
const transactionIndex: number = 0;

const assetEvent: AssetCreateEventV1 = defaultAssetCreateEvent;
Expand Down
3 changes: 0 additions & 3 deletions indexer/services/ender/__tests__/handlers/perpetual-market-handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
MarketTable,
Ordering,
testConstants,
marketRefresher,
BlockTable,
TendermintEventTable,
perpetualMarketRefresher,
Expand Down Expand Up @@ -115,7 +114,6 @@ describe('perpetualMarketHandler', () => {

it('fails when liquidity tier doesnt exist for perpetual market', async () => {
await MarketTable.create(testConstants.defaultMarket);
await marketRefresher.updateMarkets();
const transactionIndex: number = 0;
const kafkaMessage: KafkaMessage = createKafkaMessageFromPerpetualMarketEvent({
perpetualMarketEvent: defaultPerpetualMarketCreateEvent,
Expand All @@ -134,7 +132,6 @@ describe('perpetualMarketHandler', () => {
LiquidityTiersTable.create(testConstants.defaultLiquidityTier),
]);
await liquidityTierRefresher.updateLiquidityTiers();
await marketRefresher.updateMarkets();

const transactionIndex: number = 0;

Expand Down
4 changes: 0 additions & 4 deletions indexer/services/ender/__tests__/lib/on-message.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
dbHelpers,
IsoString,
LiquidityTiersTable,
marketRefresher,
MarketTable,
perpetualMarketRefresher,
PerpetualMarketTable,
Expand Down Expand Up @@ -893,13 +892,11 @@ describe('on-message', () => {
// Initialize assetRefresher
await assetRefresher.updateAssets();
await perpetualMarketRefresher.updatePerpetualMarkets();
await marketRefresher.updateMarkets();
(SubaccountUpdateHandler as jest.Mock).mockReturnValue({
handle: () => {
// clear cache so we can confirm that the cache is updated after the error
assetRefresher.clear();
perpetualMarketRefresher.clear();
marketRefresher.clear();
throw new Error();
},
validate: () => null,
Expand All @@ -909,7 +906,6 @@ describe('on-message', () => {

expect(assetRefresher.getAssetsMap()).not.toEqual({});
expect(perpetualMarketRefresher.getPerpetualMarketsMap()).not.toEqual({});
expect(marketRefresher.getMarketsMap()).not.toEqual({});
});
});

Expand Down
3 changes: 1 addition & 2 deletions indexer/services/ender/__tests__/validators/asset-validator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { logger } from '@dydxprotocol-indexer/base';
import { AssetCreateEventV1, IndexerTendermintBlock, IndexerTendermintEvent } from '@dydxprotocol-indexer/v4-protos';
import { dbHelpers, marketRefresher, testMocks } from '@dydxprotocol-indexer/postgres';
import { dbHelpers, testMocks } from '@dydxprotocol-indexer/postgres';
import { DydxIndexerSubtypes } from '../../src/lib/types';
import {
defaultAssetCreateEvent, defaultHeight, defaultTime, defaultTxHash,
Expand All @@ -15,7 +15,6 @@ import { AssetValidator } from '../../src/validators/asset-validator';
describe('asset-validator', () => {
beforeEach(async () => {
await testMocks.seedData();
await marketRefresher.updateMarkets();
jest.spyOn(logger, 'error');
});

Expand Down
3 changes: 1 addition & 2 deletions indexer/services/ender/__tests__/validators/perpetual-market-validator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { logger, ParseMessageError } from '@dydxprotocol-indexer/base';
import { PerpetualMarketCreateEventV1, IndexerTendermintBlock, IndexerTendermintEvent } from '@dydxprotocol-indexer/v4-protos';
import {
dbHelpers, marketRefresher, testMocks, perpetualMarketRefresher,
dbHelpers, testMocks, perpetualMarketRefresher,
Copy link
Contributor

Choose a reason for hiding this comment

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

The import of perpetualMarketRefresher should be removed as per the summary of changes.

-  dbHelpers, testMocks, perpetualMarketRefresher,
+  dbHelpers, testMocks,
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
dbHelpers, testMocks, perpetualMarketRefresher,
dbHelpers, testMocks,

} from '@dydxprotocol-indexer/postgres';
import { DydxIndexerSubtypes } from '../../src/lib/types';
import {
Comment on lines 1 to 7
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [24-24]

The call to perpetualMarketRefresher.clear() in the afterEach hook should be removed as it is no longer necessary with the removal of the market cache functionality.

-    await perpetualMarketRefresher.clear();

Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [34-34]

The call to perpetualMarketRefresher.updatePerpetualMarkets() in the test case for 'throws error on existing perpetual market' should be removed as it is no longer necessary with the removal of the market cache functionality.

-      await perpetualMarketRefresher.updatePerpetualMarkets();

Expand All @@ -18,7 +18,6 @@ import Long from 'long';
describe('perpetual-market-validator', () => {
beforeEach(async () => {
await testMocks.seedData();
await marketRefresher.updateMarkets();
jest.spyOn(logger, 'error');
});

Expand Down
2 changes: 0 additions & 2 deletions indexer/services/ender/src/caches/block-cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
IsolationLevel,
Transaction,
assetRefresher,
marketRefresher,
perpetualMarketRefresher,
} from '@dydxprotocol-indexer/postgres';
import Big from 'big.js';
Expand Down Expand Up @@ -111,7 +110,6 @@ export async function initializeAllCaches(): Promise<void> {
startCandleCache(txId),
perpetualMarketRefresher.updatePerpetualMarkets({ txId }),
assetRefresher.updateAssets({ txId }),
marketRefresher.updateMarkets({ txId }),
]);

await Transaction.rollback(txId);
Copy link
Contributor

Choose a reason for hiding this comment

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

The transaction in initializeAllCaches is being rolled back at the end of the function. This would undo any updates made by the refresh functions. If this is not intentional, it should be replaced with a commit.

-  await Transaction.rollback(txId);
+  await Transaction.commit(txId);
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
await Transaction.rollback(txId);
await Transaction.commit(txId);

Expand Down
9 changes: 1 addition & 8 deletions indexer/services/ender/src/handlers/markets/market-create-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
import { logger } from '@dydxprotocol-indexer/base';
import {
MarketFromDatabase,
MarketModel,
marketRefresher,
storeHelpers,
} from '@dydxprotocol-indexer/postgres';
import { MarketEventV1 } from '@dydxprotocol-indexer/v4-protos';
import * as pg from 'pg';

import { ConsolidatedKafkaEvent, MarketCreateEventMessage } from '../../lib/types';
import { Handler } from '../handler';
Expand All @@ -27,7 +23,7 @@ export class MarketCreateHandler extends Handler<MarketEventV1> {
});

const eventDataBinary: Uint8Array = this.indexerTendermintEvent.dataBytes;
const result: pg.QueryResult = await storeHelpers.rawQuery(
await storeHelpers.rawQuery(
`SELECT dydx_market_create_handler(
'${JSON.stringify(MarketEventV1.decode(eventDataBinary))}'
) AS result;`,
Expand All @@ -50,9 +46,6 @@ export class MarketCreateHandler extends Handler<MarketEventV1> {
throw error;
});

const market: MarketFromDatabase = MarketModel.fromJson(
result.rows[0].result.market) as MarketFromDatabase;
marketRefresher.updateMarket(market);
return [];
}
}
10 changes: 2 additions & 8 deletions indexer/services/ender/src/handlers/markets/market-modify-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import { logger } from '@dydxprotocol-indexer/base';
import {
MarketFromDatabase, marketRefresher, storeHelpers, MarketModel,
} from '@dydxprotocol-indexer/postgres';
import { storeHelpers } from '@dydxprotocol-indexer/postgres';
import { MarketEventV1 } from '@dydxprotocol-indexer/v4-protos';
import * as pg from 'pg';

import { ConsolidatedKafkaEvent, MarketModifyEventMessage } from '../../lib/types';
import { Handler } from '../handler';
Expand All @@ -24,7 +21,7 @@ export class MarketModifyHandler extends Handler<MarketEventV1> {
});

const eventDataBinary: Uint8Array = this.indexerTendermintEvent.dataBytes;
const result: pg.QueryResult = await storeHelpers.rawQuery(
await storeHelpers.rawQuery(
`SELECT dydx_market_modify_handler(
'${JSON.stringify(MarketEventV1.decode(eventDataBinary))}'
) AS result;`,
Expand Down Expand Up @@ -52,9 +49,6 @@ export class MarketModifyHandler extends Handler<MarketEventV1> {
);
});

const market: MarketFromDatabase = MarketModel.fromJson(
result.rows[0].result.market) as MarketFromDatabase;
marketRefresher.updateMarket(market);
return [];
}
}
4 changes: 1 addition & 3 deletions indexer/services/ender/src/handlers/markets/market-price-update-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
MarketFromDatabase,
OraclePriceFromDatabase,
OraclePriceModel,
MarketMessageContents, storeHelpers, MarketModel, marketRefresher,
MarketMessageContents, storeHelpers, MarketModel,
} from '@dydxprotocol-indexer/postgres';
import { MarketEventV1 } from '@dydxprotocol-indexer/v4-protos';
import * as pg from 'pg';
Expand Down Expand Up @@ -62,8 +62,6 @@ export class MarketPriceUpdateHandler extends Handler<MarketEventV1> {
const oraclePrice: OraclePriceFromDatabase = OraclePriceModel.fromJson(
result.rows[0].result.oracle_price) as OraclePriceFromDatabase;

marketRefresher.updateMarket(market);

return [
this.generateKafkaEvent(
oraclePrice, market.pair,
Expand Down
3 changes: 1 addition & 2 deletions indexer/services/ender/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { logger, startBugsnag, wrapBackgroundTask } from '@dydxprotocol-indexer/base';
import { stopConsumer, startConsumer } from '@dydxprotocol-indexer/kafka';
import {
assetRefresher, perpetualMarketRefresher, marketRefresher, liquidityTierRefresher,
assetRefresher, perpetualMarketRefresher, liquidityTierRefresher,
} from '@dydxprotocol-indexer/postgres';

import { initializeAllCaches } from './caches/block-cache';
Expand All @@ -24,7 +24,6 @@ async function startKafka(): Promise<void> {
await Promise.all([
perpetualMarketRefresher.updatePerpetualMarkets(),
assetRefresher.updateAssets(),
marketRefresher.updateMarkets(),
liquidityTierRefresher.updateLiquidityTiers(),
]);
// Ender does not need to refresh its caches in a loop because Ender is the only service that
Expand Down
2 changes: 0 additions & 2 deletions indexer/services/ender/src/lib/cache-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {
IsolationLevel,
Transaction,
assetRefresher,
marketRefresher,
perpetualMarketRefresher,
} from '@dydxprotocol-indexer/postgres';

Expand All @@ -13,7 +12,6 @@ export async function refreshDataCaches(): Promise<void> {
await Promise.all([
perpetualMarketRefresher.updatePerpetualMarkets({ txId, readReplica: true }),
assetRefresher.updateAssets({ txId, readReplica: true }),
marketRefresher.updateMarkets({ txId, readReplica: true }),
]);

await Transaction.rollback(txId);
Copy link
Contributor

Choose a reason for hiding this comment

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

The refreshDataCaches function ends with a rollback, which undoes any changes made within the transaction. This could be an error unless intentionally designed to simulate the updates without persisting them. Please verify the intended behavior.

-  await Transaction.rollback(txId);
+  await Transaction.commit(txId);
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
await Transaction.rollback(txId);
await Transaction.commit(txId);

Expand Down