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

CCIP-2206 Added support for retention in the contract_transmitter.go #12998

Merged
merged 2 commits into from Apr 30, 2024

Conversation

mateusz-sekara
Copy link
Collaborator

@mateusz-sekara mateusz-sekara commented Apr 26, 2024

contract_transmitter enables filters in LogPoller that stream all the Transmit events from the chain to the database. Only to pick the latest one from the database.

The following db query is used for that (SelectLatestLogByEventSigWithConfs):

SELECT * FROM evm.logs
			WHERE evm_chain_id = :evm_chain_id
			AND event_sig = :event_sig
			AND address = :address
			AND block_number <= %s
			ORDER BY (block_number, log_index) DESC LIMIT 1

Unfortunately, the more transmits on the lane/ocr2 contract, the more logs we keep in the database only to pick the latest one for getting the current epoch and configDigest. This grows indefinitely because logs are never evicted from the database. Also, the query needs to gather all Transmit logs from a single contract, sort them, and pick the latest one.

Example plan on prod-testnet example

 Limit  (cost=17989.55..17989.55 rows=1 width=416) (actual time=597.180..597.183 rows=1 loops=1)
   Buffers: shared hit=37091 read=506
   I/O Timings: read=510.496
   InitPlan 1 (returns $0)
     ->  Limit  (cost=0.43..0.48 rows=1 width=16) (actual time=1.821..1.822 rows=1 loops=1)
           Buffers: shared hit=4 read=1
           I/O Timings: read=1.746
           ->  Index Only Scan using idx_evm_log_poller_blocks_order_by_block on log_poller_blocks  (cost=0.43..4380.14 rows=78039 width=16) (actual time=1.820..1.820 rows=1 loops=1)
                 Index Cond: (evm_chain_id = '43113'::numeric)
                 Heap Fetches: 1
                 Buffers: shared hit=4 read=1
                 I/O Timings: read=1.746
   ->  Sort  (cost=17989.06..18001.47 rows=4963 width=416) (actual time=597.179..597.179 rows=1 loops=1)
         Sort Key: (ROW(logs.block_number, logs.log_index)) DESC
         Sort Method: top-N heapsort  Memory: 26kB
         Buffers: shared hit=37091 read=506
         I/O Timings: read=510.496
         ->  Index Scan using idx_evm_logs_ordered_by_block_and_created_at on logs  (cost=0.56..17964.25 rows=4963 width=416) (actual time=4.768..573.746 rows=39707 loops=1)
               Index Cond: ((evm_chain_id = '43113'::numeric) AND (address = '\x3e0df884042c21e83276abb368b4388a17f78a82'::bytea) AND (event_sig = '\xb04e63db38c49950639fa09d29872f21f5d49d614f3a969d8adf3d4b52e41a62'::bytea) AND (block_number <= $0))
               Buffers: shared hit=37091 read=506
               I/O Timings: read=510.496
 Planning Time: 0.260 ms
 Execution Time: 597.230 ms
(23 rows)

The first step is to enable products to pass retention for that filter (each product can pass whatever the value is accurate for them). That way LogPoller will be able to keep this dataset bounded. Ideally, we should pass MaxLogsKept = 1, but it's not supported by LP yet.

Solution

Copy link
Contributor

I see you updated files related to core. Please run pnpm changeset in the root directory to add a changeset as well as in the text include at least one of the following tags:

  • #added For any new functionality added.
  • #breaking_change For any functionality that requires manual action for the node to boot.
  • #bugfix For bug fixes.
  • #changed For any change to the existing functionality.
  • #db_update For any feature that introduces updates to database schema.
  • #deprecation_notice For any upcoming deprecation functionality.
  • #internal For changesets that need to be excluded from the final changelog.
  • #nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
  • #removed For any functionality/config that is removed.
  • #updated For any functionality that is updated.
  • #wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

Copy link
Contributor

I see you added a changeset file but it does not contain a tag. Please edit the text include at least one of the following tags:

  • #added For any new functionality added.
  • #breaking_change For any functionality that requires manual action for the node to boot.
  • #bugfix For bug fixes.
  • #changed For any change to the existing functionality.
  • #db_update For any feature that introduces updates to database schema.
  • #deprecation_notice For any upcoming deprecation functionality.
  • #internal For changesets that need to be excluded from the final changelog.
  • #nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
  • #removed For any functionality/config that is removed.
  • #updated For any functionality that is updated.
  • #wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

@mateusz-sekara mateusz-sekara changed the title Added support for retention in the contract_transmitter.go CCIP-2206 Added support for retention in the contract_transmitter.go Apr 29, 2024
err := lp.RegisterFilter(ctx, logpoller.Filter{Name: transmitterFilterName(address), EventSigs: []common.Hash{transmitted.ID}, Addresses: []common.Address{address}})
// TODO It would be better to keep MaxLogsKept = 1 for the OCR contract transmitter instead of Retention. We are always interested only in the latest log.
// Although MaxLogsKept is present in the Filter struct, it is not supported by LogPoller yet.
err := lp.RegisterFilter(ctx, logpoller.Filter{Name: transmitterFilterName(address), EventSigs: []common.Hash{transmitted.ID}, Addresses: []common.Address{address}, Retention: retention})
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this was bumped way down in priority after the field was added but before the implementation was added. Shouldn't be difficult, but not sure when I'll have the time to get to it.

@mateusz-sekara mateusz-sekara added this pull request to the merge queue Apr 30, 2024
Merged via the queue into develop with commit d50936c Apr 30, 2024
105 checks passed
@mateusz-sekara mateusz-sekara deleted the contract-transmitter-retention branch April 30, 2024 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants