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

fix: kill old execution module when redeploying under same namespace and identifier #640

Merged
merged 2 commits into from
Mar 10, 2023

Conversation

deekerno
Copy link
Contributor

@deekerno deekerno commented Mar 9, 2023

Changelog

  • Kill old execution module if deploying a new execution module under the same namespace and identifier

Reproduction (on master)

  1. Ensure database is clear and build forc-index: cargo build -p forc-index.
  2. Add log message to examples/block-explorer/explorer-index/src/lib.rs; something like "FIRST" will suffice.
  3. Deploy explorer index:
../../target/debug/forc-index deploy \                                                      
   --path explorer-index \
   --output-dir-root [whatever your fuel-indexer repo root is] \
   --url http://0.0.0.0:29987 \
   --target wasm32-unknown-unknown
  1. Change log message in explorer example to something different, e.g. "SECOND".
  2. Re-deploy using same command.
  3. Search through service logs; you should see that both messages are being logged.

Testing Plan (on this branch)

Repeat the above steps, you should see that only the second message is being logged. There should also be a message saying the following:

Replaced execution module for fuel_examples.explorer_index, stopping old execution module.

@deekerno deekerno added the bug Something isn't working label Mar 9, 2023
@deekerno deekerno self-assigned this Mar 9, 2023
@deekerno deekerno marked this pull request as ready for review March 9, 2023 19:28
@deekerno deekerno requested a review from a team March 9, 2023 19:28
Copy link
Contributor

@ra0x3 ra0x3 left a comment

Choose a reason for hiding this comment

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

  • utACK
  • @deekerno nice find here 🙏🏽
  • Left a comment. We should definitely refer to this as an executor (the literal execution coroutine).
  • We should also refer to the manifest.uid() as an "indexer"
    • There are some other parts of the code we need to update with this as well but this can be done separately. Basically need to replace all references to "indexer" as "indexer" (unless it's referring to the literal data that is indexed by an indexer - which is indeed an index)

packages/fuel-indexer/src/service.rs Outdated Show resolved Hide resolved
@deekerno deekerno force-pushed the deekerno/kill-old-wasm-on-redeployed-index branch from 31d718f to 0d7e0eb Compare March 10, 2023 05:18
@deekerno deekerno enabled auto-merge (squash) March 10, 2023 05:34
@deekerno deekerno requested a review from ra0x3 March 10, 2023 05:34
@deekerno deekerno merged commit 099e202 into master Mar 10, 2023
@deekerno deekerno deleted the deekerno/kill-old-wasm-on-redeployed-index branch March 10, 2023 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants