minor improvements to faucet worker #3062
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cbeck88
commented
Jan 27, 2023
wjuan-mob
reviewed
Jan 27, 2023
wjuan-mob
previously approved these changes
Jan 28, 2023
cbeck88
commented
Jan 28, 2023
cbeck88
commented
Jan 28, 2023
|
The CD failure here seems to happen a whole lot more after we integrated the faucet. The cd-integration-tests stalls out on one of the python scripts that sets up monitors waiting for the blocks to sync. Mobilecoind doesn't show any errors when this happens. I'm guessing some kind of weirdness/race when multiple clients are trying to watch monitors? |
|
I would kind of like to lean into the faucet and not use "drain accounts"? What do you think about that? |
|
Maybe it will be better with the "activate" option? So the faucet wont do anything while drain accounts is trying to work? #3063 |
This rearranges some of the code that talks to mobilecoind in a
way that I think should reduce some racy error conditions.
It also renames a variable to be more clear, the old name doesn't
make as much sense anymore.
This was tested locally against
```
diff --git a/mobilecoind/src/sync.rs b/mobilecoind/src/sync.rs
index b78be780a..8f166fa85 100644
--- a/mobilecoind/src/sync.rs
+++ b/mobilecoind/src/sync.rs
@@ -299,6 +299,8 @@ fn sync_monitor(
let block_contents = match ledger_db.get_block_contents(monitor_data.next_block) {
Ok(block_contents) => block_contents,
Err(mc_ledger_db::Error::NotFound) => {
+ // Fake S3 delay
+ std::thread::sleep(std::time::Duration::from_secs(1));
return Ok(SyncMonitorOk::NoMoreBlocks);
}
Err(err) => {
```
to try to simulate S3 delay.
Co-authored-by: wjuan-mob <[email protected]>
I don't think this should be necessary, but I'm somewhat out of ideas. It can't hurt and may prevent an error I guess.
cbeck88
force-pushed
the
fixups-to-slam
branch
from
3b78d0c to
43511a0
Compare
nick-mobilecoin
approved these changes
Jan 31, 2023
Jason requested that, the mobilecoind-dev-faucet should not immediately start sending lots of transactions after it is deployed, because that makes some other deployment-related things harder. By default it does that because it wants to have its queue full. The change we made is: * The faucet worker now has an "in_active" flag, which if false causes it to skip its loop, and this defaults to false. * The flag is set to true if `--activate` is passed. * If `--activate` is passed, the flag is set true whenever an HTTP POST occurs (post to get payment from faucet, or to start a slam), whether or not the POST results in a successful operation. * Docu is updated so that for dev testing it demonstrates `--activate` and the existing documented flows should still work. * CI is updated to use `--activate`
wjuan-mob
reviewed
Jan 31, 2023
wjuan-mob
reviewed
Jan 31, 2023
wjuan-mob
approved these changes
Jan 31, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.