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 todays sql error and remember which machine did a task #11415

Merged
merged 4 commits into from
Nov 14, 2023

Conversation

snadrus
Copy link
Collaborator

@snadrus snadrus commented Nov 13, 2023

Related Issues

  1. Schema problem
  2. Slack comment that there's no way to figure out which machine did some task to be able to dive into its logs for errors.

Proposed Changes

  • Schema fixed, but still needs a "DROP SCHEMA lotus CASCADE".
  • harmony_task_history now remembers which hostname + port (identifier) did some piece of work.

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • If the PR affects users (e.g., new feature, bug fix, system requirements change), update the CHANGELOG.md and add details to the UNRELEASED section.
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

@snadrus snadrus requested a review from a team as a code owner November 13, 2023 23:10
@snadrus snadrus requested a review from magik6k November 13, 2023 23:11
Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

2 nits, but looks good generally

return xerrors.Errorf(`'%w' while parsing the config toml's
[Apis]
StorageRPCSecret=%v
Get it from the JSON documents in ~/.lotus-miner/keystore called .PrivateKey`, err, cfg.Apis.StorageRPCSecret)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should link specifically ~/.lotusminer/keystore/MF2XI2BNNJ3XILLQOJUXMYLUMU

@@ -0,0 +1 @@
ALTER TABLE harmony_task_history ADD COLUMN completed_by_host_and_port varchar(300) NOT NULL;
Copy link
Contributor

Choose a reason for hiding this comment

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

tbh given that we're not rolled out anywhere yet, I'd say that we can afford to modify the main schema files, we can just drop the lotus schema between test runs;

(Not a super strong opinion)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I figure it will be good to have a solid example for junior engineers.

We could go further to protect the schema state from corruption by remembering the file hash in the database and failing to start if the hash of an old file looks different.

@snadrus snadrus merged commit 3864831 into feat/sturdypost Nov 14, 2023
@snadrus snadrus deleted the remember-machine branch November 14, 2023 14:18
@snadrus
Copy link
Collaborator Author

snadrus commented Nov 14, 2023

@Reiers This is merged, and we are ready to continue testing.

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.

2 participants