close resources when shutting down the schema registry server

[junrao]

Now that we have the clean shutdown support in rest utils (confluentinc/rest-utils#2), we can add the shutdown hook in schema registry server to close resources cleanly.

We also need to add the signal handler in Main so that we can shutdown the server cleanly on ctrl-C.

[ewencp]

Jetty actually catches the Ctrl+C for you, so you should just need to implement the onShutdown hook.

[blbradley]

I believe this issue is still relevant due to #197. If I try to use RestApp.start twice in the same test suite, I get java.net.BindException: Address already in use. I am using RestApp.stop at the end of the test.

[ewencp]

@blbradley This repo uses RestApp in test suites successfully, see https://github.com/confluentinc/schema-registry/blob/master/core/src/test/java/io/confluent/kafka/schemaregistry/ClusterTestHarness.java Is it possible you are using a fixed port and running tests in parallel? That is a common source of Address already in use errors. Even without that, you can potentially hit port conflicts since there is no reliable way to "reserve" a port in a test before you have actually bound it. (This is why Kafka tests have mostly moved to using 0 as the port and passing the port to downstream apps after the port is bound.)

[blbradley]

@ewencp

Is it possible you are using a fixed port and running tests in parallel?

I am using a fixed port and using parallelExecution in Test := false in my sbt build file. So, no tests are running in parallel.

Even without that, you can potentially hit port conflicts since there is no reliable way to "reserve" a port in a test before you have actually bound it.

Indeed. I get Address already in use when calls happen in this order:

{
  val restApp = RestApp( ... )
  restApp.start
  // tests and assertions
  restApp.stop  
}

{
  val restApp = RestApp( ... )
  restApp.start // java.net.BindException: Address already in use
}

Is this supposed to happen? Shouldn't RestApp.stop() unbind the port?

[ewencp]

@blbradley Yes, that should work unless the kernel isn't allowing the port to be used again for some reason. RestApp.stop() definitely closes the socket. Although to avoid using a fixed port (which is often error prone since other apps could also rely on that port and we've often found people have accidental conflicts), you can also just choose a port randomly for each test to try to isolate them. It is not foolproof, but we use this simple method to choose ports.

[blbradley]

@ewencp I use scalatest-embedded-kafka with no problems. Kafka and Zookeeper are started and stopped for each test within my project. Each on the same port per service per test. So I don't think it is a kernel issue.

[blbradley]

I have since removed Kafka integration tests from my project. I believe my issue was due to not cleaning up probably after a test fails.

@ewencp Thanks for your quick responses to my issues. Discussion is always appreciated.