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

Disable backup handler in tests #1399

Merged
merged 2 commits into from
Apr 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .semaphore/semaphore.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ blocks:
jobs:
- name: Build & Test
commands:
# configure file limits
- ulimit -S -n 1024000
- echo "fs.file-max = 1024000" | sudo tee -a /etc/sysctl.conf
Comment on lines +24 to +26
Copy link
Member

Choose a reason for hiding this comment

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

Now that we have fixed the test issues in #1400 I wonder if this is really necessary. If it is, why do the test pass? Shouldn't the "too many open files" error completely break the tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't the "too many open files" error completely break the tests?

It looks like it does, but it makes our test suite hang so we don't cleanly witness it (because we kill the build after 15 minutes to avoid wasting resources).

I wonder if this is really necessary

I can't be entirely sure, but since I added that I haven't repro-ed the hang on semaphore (I restarted the test suite several times), so it looks like it happens less often (hopefully it doesn't happen anymore). At least it can't hurt to raise this limit (the ulimit is already at 1024000 by default in semaphore, but it wasn't configured in sysctl.conf).

Copy link
Member

Choose a reason for hiding this comment

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

Ok, so this fixes the intermittent hanging while #1400 introduced then fixed a permanent hanging.

- sem-version java 11
- checkout
- cache restore maven
Expand Down
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ before_install:
- export M2_HOME=$PWD/apache-maven-3.6.3
- export PATH=$M2_HOME/bin:$PATH
script:
- echo "fs.file-max = 1024000" | sudo tee -a /etc/sysctl.conf
- mvn scoverage:report
cache:
directories:
Expand Down
1 change: 1 addition & 0 deletions eclair-core/src/main/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ eclair {
password = "" // password for basic auth, must be non empty if json-rpc api is enabled
}

enable-db-backup = true // enable the automatic sqlite db backup; do not change this unless you know what you are doing
// override this with a script/exe that will be called everytime a new database backup has been created
# backup-notify-script = "/absolute/path/to/script.sh"

Expand Down
26 changes: 16 additions & 10 deletions eclair-core/src/main/scala/fr/acinq/eclair/Setup.scala
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import scala.concurrent.duration._
* @param datadir directory where eclair-core will write/read its data.
* @param overrideDefaults use this parameter to programmatically override the node configuration .
* @param seed_opt optional seed, if set eclair will use it instead of generating one and won't create a seed.dat file.
* @param db optional databases to use, if not set eclair will create the necessary databases
*/
class Setup(datadir: File,
overrideDefaults: Config = ConfigFactory.empty(),
Expand Down Expand Up @@ -117,8 +118,10 @@ class Setup(datadir: File,
val feeratesPerKw = new AtomicReference[FeeratesPerKw](null)

val feeEstimator = new FeeEstimator {
// @formatter:off
override def getFeeratePerKb(target: Int): Long = feeratesPerKB.get().feePerBlock(target)
override def getFeeratePerKw(target: Int): Long = feeratesPerKw.get().feePerBlock(target)
// @formatter:on
}

val nodeParams = NodeParams.makeNodeParams(config, keyManager, initTor(), database, blockCount, feeEstimator)
Expand Down Expand Up @@ -153,7 +156,7 @@ class Setup(datadir: File,
blocks = (json \ "blocks").extract[Long]
headers = (json \ "headers").extract[Long]
chainHash <- bitcoinClient.invoke("getblockhash", 0).map(_.extract[String]).map(s => ByteVector32.fromValidHex(s)).map(_.reverse)
bitcoinVersion <- bitcoinClient.invoke("getnetworkinfo").map(json => (json \ "version")).map(_.extract[Int])
bitcoinVersion <- bitcoinClient.invoke("getnetworkinfo").map(json => json \ "version").map(_.extract[Int])
unspentAddresses <- bitcoinClient.invoke("listunspent").collect { case JArray(values) =>
values
.filter(value => (value \ "spendable").extract[Boolean])
Expand Down Expand Up @@ -270,17 +273,20 @@ class Setup(datadir: File,
implicit val timeout = Timeout(30 seconds)
new ElectrumEclairWallet(electrumWallet, nodeParams.chainHash)
}
_ = wallet.getFinalAddress.map {
case address => logger.info(s"initial wallet address=$address")
}
_ = wallet.getFinalAddress.map(address => logger.info(s"initial wallet address=$address"))
// do not change the name of this actor. it is used in the configuration to specify a custom bounded mailbox

backupHandler = system.actorOf(SimpleSupervisor.props(
BackupHandler.props(
nodeParams.db,
new File(chaindir, "eclair.sqlite.bak"),
if (config.hasPath("backup-notify-script")) Some(config.getString("backup-notify-script")) else None
), "backuphandler", SupervisorStrategy.Resume))
backupHandler = if (config.getBoolean("enable-db-backup")) {
system.actorOf(SimpleSupervisor.props(
BackupHandler.props(
nodeParams.db,
new File(chaindir, "eclair.sqlite.bak"),
if (config.hasPath("backup-notify-script")) Some(config.getString("backup-notify-script")) else None
pm47 marked this conversation as resolved.
Show resolved Hide resolved
), "backuphandler", SupervisorStrategy.Resume))
} else {
logger.warn("database backup is disabled")
system.deadLetters
}
audit = system.actorOf(SimpleSupervisor.props(Auditor.props(nodeParams), "auditor", SupervisorStrategy.Resume))
register = system.actorOf(SimpleSupervisor.props(Props(new Register), "register", SupervisorStrategy.Resume))
commandBuffer = system.actorOf(SimpleSupervisor.props(Props(new CommandBuffer(nodeParams, register)), "command-buffer", SupervisorStrategy.Resume))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class IntegrationSpec extends TestKit(ActorSystem("test")) with BitcoindService

val commonConfig = ConfigFactory.parseMap(Map(
"eclair.chain" -> "regtest",
"eclair.enable-db-backup" -> false,
"eclair.server.public-ips.1" -> "127.0.0.1",
"eclair.bitcoind.port" -> bitcoindPort,
"eclair.bitcoind.rpcport" -> bitcoindRpcPort,
Expand Down