Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Switching sccache from local to Redis #10971

Merged
merged 5 commits into from
Oct 2, 2019
Merged

Switching sccache from local to Redis #10971

merged 5 commits into from
Oct 2, 2019

Conversation

General-Beck
Copy link
Contributor

@General-Beck General-Beck commented Aug 14, 2019

All the images which are used in parity-ethereum build process are now being published to our Gitilab docker container registry.
These images now are making use of sccache with Redis to avoid the data race condition when pruning old cache pieces.
Metadata was also edited to show the current addresses of the images.
test

Depends on paritytech/scripts#91

@General-Beck General-Beck added A0-pleasereview 🤓 Pull request needs code review. M1-ci 🙉 Continuous integration. labels Aug 14, 2019
@General-Beck General-Beck self-assigned this Aug 14, 2019
@ordian ordian changed the title [WIP] Switching sccache from local to Radis [WIP] Switching sccache from local to Redis Aug 14, 2019
.gitlab-ci.yml Outdated
echo "_____No logs from sccache_____";
exit 0;
fi
- SCCACHE_REDIS=redis://:${REDIS}@172.17.0.1/0 sccache --start-server
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it wise to use the explicit ip address here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a default gateway address of a Docker container-internal network (so in practice it's a mapping of host's 127.0.0.1 into a local subnet).
I guess it's safe enough, provided that we're not going to mess with Docker's networking too much in this setup.

As far as I understood from my chat with @General-Beck earlier today, there's no easy way to teach sccache to identify that IP by hostname, and there's no non-hacky lookup flow to be used instead either.

Copy link
Collaborator

@TriplEight TriplEight left a comment

Choose a reason for hiding this comment

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

Everything must work after images from paritytech/scripts#91 will be in the registry.

Copy link
Contributor

@gabreal gabreal left a comment

Choose a reason for hiding this comment

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

I'm really a big fan of breaking one thing at a time.

One pr for new docker registry and another for changing the sccache makes it easier to exclude errors in the future.

Would be good to remove the wip from the title when requesting reviews.

looks good apart from that 👍

else
echo "_____No logs from sccache_____";
exit 0;
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

so debugging will be disabled completely?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, sccache debugging was actually helpful only once.
But what's important and bugs me, is that debug log level somehow hinders sccache to communicate with redis.
Also, had to remove sccache -s as it was failing the job because it couldn't get the info from Redis. I tried some ideas, but think that in order to do that, we should install sccache to CI1 and CI2 which is not really necessary, at least for now.

@TriplEight TriplEight changed the title [WIP] Switching sccache from local to Redis Switching sccache from local to Redis Aug 15, 2019
@dvdplm
Copy link
Collaborator

dvdplm commented Sep 6, 2019

@General-Beck what is the status of this? Do you need any help sorting out the tests?

@General-Beck
Copy link
Contributor Author

@dvdplm I do the latest checks and tests. I think tomorrow we will launch

@General-Beck General-Beck added the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Sep 12, 2019
Copy link
Collaborator

@TriplEight TriplEight left a comment

Choose a reason for hiding this comment

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

👍 ]

@General-Beck General-Beck merged commit 0bd2979 into master Oct 2, 2019
@ordian ordian deleted the sccache-redis branch October 2, 2019 20:01
@ordian ordian added this to the 2.7 milestone Oct 2, 2019
@ordian ordian added A8-looksgood 🦄 Pull request is reviewed well. B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. B1-patch-beta 🕷🕷 and removed A0-pleasereview 🤓 Pull request needs code review. A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Oct 2, 2019
@soc1c soc1c mentioned this pull request Nov 6, 2019
36 tasks
@soc1c soc1c mentioned this pull request Nov 6, 2019
35 tasks
dvdplm pushed a commit that referenced this pull request Nov 6, 2019
* Switching cache from local to Radis

* sccache -s won't work. All the images are taken from own registry

* new images addresses

* statisticss after stop sccache server
sccache CC&CXX
niklasad1 pushed a commit that referenced this pull request Nov 6, 2019
* Switching cache from local to Radis

* sccache -s won't work. All the images are taken from own registry

* new images addresses

* statisticss after stop sccache server
sccache CC&CXX
s3krit pushed a commit that referenced this pull request Nov 11, 2019
* ropsten #6631425 foundation #8798209 (#11201)
* [stable] builtin, istanbul and mordor testnet backports (#11234)
  * ethcore-builtin (#10850)
  * [builtin]: support `multiple prices and activations` in chain spec (#11039)
  * [chain specs]: activate `Istanbul` on mainnet (#11228)
  * ethcore/res: add mordor testnet configuration (#11200)
* Update list of bootnodes for xDai chain (#11236)
* ethcore: remove `test-helper feat` from build (#11047)
* Secret store: fix Instant::now() related race in net_keep_alive (#11155) (#11159)
* [stable]: backport #10691 and #10683 (#11143)
  * Fix compiler warning (that will become an error) (#10683)
  * Refactor Clique stepping (#10691)
* Add Constantinople eips to the dev (instant_seal) config (#10809)
* Add cargo-remote dir to .gitignore (?)
* Insert explicit warning into the panic hook (#11225)
* Fix docker centos build (#11226)
* Update MIX bootnodes. (#11203)
* Use provided usd-per-eth value if an endpoint is specified (#11209)
* Add new line after writing block to hex file. (#10984)
* Type annotation for next_key() matching of json filter options (#11192) (but no `FilterOption` in 2.5 so…)
* Upgrade jsonrpc to latest (#11206)
* [CI] check evmbin build (#11096)
* Correct EIP-712 encoding (#11092)
* [client]: Fix for incorrectly dropped consensus messages (#11086)
* Fix block detail updating (#11015)
* Switching sccache from local to Redis (#10971)
* Made ecrecover implementation trait public (#11188)
* [dependencies]: jsonrpc `14.0.1` (#11183)
* [receipt]: add `sender` & `receiver` to `RichReceipts` (#11179)
* [ethcore/builtin]: do not panic in blake2pricer on short input (#11180)
* util Host: fix a double Read Lock bug in fn Host::session_readable() (#11175)
* ethcore client: fix a double Read Lock bug in fn Client::logs() (#11172)
* Change how RPCs eth_call and eth_estimateGas handle "Pending" (#11127)
* Cleanup stratum a bit (#11161)
* Upgrade to jsonrpc v14 (#11151)
* SecretStore: expose restore_key_public in HTTP API (#10241)
s3krit pushed a commit that referenced this pull request Nov 11, 2019
* [CI] check evmbin build (#11096)
* Correct EIP-712 encoding (#11092)
* [client]: Fix for incorrectly dropped consensus messages (#11082) (#11086)
* Update hardcoded headers (foundation, classic, kovan, xdai, ewc, ...) (#11053)
* Add cargo-remote dir to .gitignore (?)
* Update light client headers: ropsten 6631425 foundation 8798209 (#11201)
* Update list of bootnodes for xDai chain (#11236)
* ethcore/res: add mordor testnet configuration (#11200)
* [chain specs]: activate Istanbul on mainnet (#11228)
* [builtin]: support multiple prices and activations in chain spec (#11039)
* [receipt]: add sender & receiver to RichReceipts (#11179)
* [ethcore/builtin]: do not panic in blake2pricer on short input (#11180)
* Made ecrecover implementation trait public (#11188)
* Fix docker centos build (#11226)
* Update MIX bootnodes. (#11203)
* Insert explicit warning into the panic hook (#11225)
* Use provided usd-per-eth value if an endpoint is specified (#11209)
* Cleanup stratum a bit (#11161)
* Add Constantinople EIPs to the dev (instant_seal) config (#10809) (already backported)
* util Host: fix a double Read Lock bug in fn Host::session_readable() (#11175)
* ethcore client: fix a double Read Lock bug in fn Client::logs() (#11172)
* Type annotation for next_key() matching of json filter options (#11192)
* Upgrade jsonrpc to latest (#11206)
* [dependencies]: jsonrpc 14.0.1 (#11183)
* Upgrade to jsonrpc v14 (#11151)
* Switching sccache from local to Redis (#10971)
* Snapshot restoration overhaul (#11219)
* Add new line after writing block to hex file. (#10984)
* Pause pruning while snapshotting (#11178)
* Change how RPCs eth_call and eth_estimateGas handle "Pending" (#11127)
* Fix block detail updating (#11015)
* Make InstantSeal Instant again #11186
* Filter out some bad ropsten warp snapshots (#11247)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. M1-ci 🙉 Continuous integration.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants