ParrotServerImpl::start doesn't call config.recordProcessor.start()

[julianrendell]

Looking at the recordprocessor trait (https://github.com/twitter/iago/blob/master/src/main/scala/com/twitter/parrot/processor/RecordProcessor.scala) I assume that that start method is intended to be called before the test starts, and shutdown after.

In playing with the example code I can see code I add to shutdown being executed but not so for start.

Looking at ParrotServerImpl (https://github.com/twitter/iago/blob/master/src/main/scala/com/twitter/parrot/server/ParrotServer.scala) and comparing start to shutdown:

 def start(): Future[Void] = {
    status.setStatus(ParrotState.STARTING)
    transport.start(config)
    queue.start()
    thriftServer.start(this, config.parrotPort)
    clusterService.start(config.parrotPort)
    status.setStatus(ParrotState.RUNNING)
    log.info("using record processor %s", config.recordProcessor.getClass().getName())
    Future.Void
  }

  def shutdown(): Future[Void] = {

    /* Calling ServiceTracker.shutdown() causes all the Ostrich threads to go away. It also results
     * in all its managed services to be shutdown. That includes this service. We put a guard
     * here so we don't end up calling ourselves twice.
     */

    if (isShutdown.compareAndSet(false, true)) {
      Future {
        status.setStatus(ParrotState.STOPPING)
        log.trace("server: shutting down")
        clusterService.shutdown()
        queue.shutdown()
        shutdownTransport
        status.setStatus(ParrotState.SHUTDOWN)
        config.recordProcessor.shutdown()
        ServiceTracker.shutdown()
        thriftServer.shutdown()
        log.trace("server: shut down")
        done.setValue(null)
      }
    }
    done
  }

then I think start should look like:

 def start(): Future[Void] = {
    status.setStatus(ParrotState.STARTING)

    config.recordProcessor.start()

    transport.start(config)
    queue.start()
    thriftServer.start(this, config.parrotPort)
    clusterService.start(config.parrotPort)
    status.setStatus(ParrotState.RUNNING)
    log.info("using record processor %s", config.recordProcessor.getClass().getName())
    Future.Void
  }

And in shutdown should the status.setStatus(ParrotState.SHUTDOWN) be after config.recordProcessor.shutdown()? (Maybe not if it is strictly indicating the parrot system's state and excluding the user supplied load test logic.)

My apologies for this not being a pull request, but I'm completely new to Iago, Scala, and Maven; I've yet to figure out how to use a local custom version of Iago from my own test project.

Thanks in advance,

Julian

[WamBamBoozle]

Julian

You're absolutely right about start. Please provide a patch.

As for where we set the state to shutdown, I don't think it matters.

On Wed, Jan 15, 2014 at 12:10 PM, Julian Rendell notifications@github.comwrote:

Looking at the recordprocessor trait ( https://github.com/twitter/iago/blob/master/src/main/scala/com/twitter/parrot/processor/RecordProcessor.scala) I assume that that start method is intended to be called before the test starts, and shutdown after.

In playing with the example code I can see code I add to shutdown being executed but not so for start.

Looking at ParrotServerImpl ( https://github.com/twitter/iago/blob/master/src/main/scala/com/twitter/parrot/server/ParrotServer.scala) and comparing start to shutdown:

def start(): Future[Void] = { status.setStatus(ParrotState.STARTING) transport.start(config) queue.start() thriftServer.start(this, config.parrotPort) clusterService.start(config.parrotPort) status.setStatus(ParrotState.RUNNING) log.info("using record processor %s", config.recordProcessor.getClass().getName()) Future.Void }

def shutdown(): Future[Void] = {

/* Calling ServiceTracker.shutdown() causes all the Ostrich threads to go away. It also results
 * in all its managed services to be shutdown. That includes this service. We put a guard
 * here so we don't end up calling ourselves twice.
 */

if (isShutdown.compareAndSet(false, true)) {
  Future {
    status.setStatus(ParrotState.STOPPING)
    log.trace("server: shutting down")
    clusterService.shutdown()
    queue.shutdown()
    shutdownTransport
    status.setStatus(ParrotState.SHUTDOWN)
    config.recordProcessor.shutdown()
    ServiceTracker.shutdown()
    thriftServer.shutdown()
    log.trace("server: shut down")
    done.setValue(null)
  }
}
done

}

then I think start should look like:

def start(): Future[Void] = { status.setStatus(ParrotState.STARTING)

config.recordProcessor.start()

transport.start(config)
queue.start()
thriftServer.start(this, config.parrotPort)
clusterService.start(config.parrotPort)
status.setStatus(ParrotState.RUNNING)
log.info("using record processor %s", config.recordProcessor.getClass().getName())
Future.Void

}

And in shutdown should the status.setStatus(ParrotState.SHUTDOWN) be after config.recordProcessor.shutdown()? (Maybe not if it is strictly indicating the parrot system's state and excluding the user supplied load test logic.)

My apologies for this not being a pull request, but I'm completely new to Iago, Scala, and Maven; I've yet to figure out how to use a local custom version of Iago from my own test project.

Thanks in advance,

Julian

— Reply to this email directly or view it on GitHubhttps://github.com/twitter/iago/issues/19 .

[WamBamBoozle]

The call

config.recordProcessor.start()

should happen right after

log.info("using record processor %s",

config.recordProcessor.getClass().getName())

On Wed, Jan 15, 2014 at 2:18 PM, Tom Howland thowland@twitter.com wrote:

Julian

You're absolutely right about start. Please provide a patch.

As for where we set the state to shutdown, I don't think it matters.

On Wed, Jan 15, 2014 at 12:10 PM, Julian Rendell <notifications@github.com

wrote:

Looking at the recordprocessor trait ( https://github.com/twitter/iago/blob/master/src/main/scala/com/twitter/parrot/processor/RecordProcessor.scala) I assume that that start method is intended to be called before the test starts, and shutdown after.

In playing with the example code I can see code I add to shutdown being executed but not so for start.

Looking at ParrotServerImpl ( https://github.com/twitter/iago/blob/master/src/main/scala/com/twitter/parrot/server/ParrotServer.scala) and comparing start to shutdown:

def start(): Future[Void] = { status.setStatus(ParrotState.STARTING) transport.start(config) queue.start() thriftServer.start(this, config.parrotPort) clusterService.start(config.parrotPort) status.setStatus(ParrotState.RUNNING) log.info("using record processor %s", config.recordProcessor.getClass().getName()) Future.Void }

def shutdown(): Future[Void] = {

/* Calling ServiceTracker.shutdown() causes all the Ostrich threads to go away. It also results
 * in all its managed services to be shutdown. That includes this service. We put a guard
 * here so we don't end up calling ourselves twice.
 */

if (isShutdown.compareAndSet(false, true)) {
  Future {
    status.setStatus(ParrotState.STOPPING)
    log.trace("server: shutting down")
    clusterService.shutdown()
    queue.shutdown()
    shutdownTransport
    status.setStatus(ParrotState.SHUTDOWN)
    config.recordProcessor.shutdown()
    ServiceTracker.shutdown()
    thriftServer.shutdown()
    log.trace("server: shut down")
    done.setValue(null)
  }
}
done

}

then I think start should look like:

def start(): Future[Void] = { status.setStatus(ParrotState.STARTING)

config.recordProcessor.start()

transport.start(config)
queue.start()
thriftServer.start(this, config.parrotPort)
clusterService.start(config.parrotPort)
status.setStatus(ParrotState.RUNNING)
log.info("using record processor %s", config.recordProcessor.getClass().getName())
Future.Void

}

And in shutdown should the status.setStatus(ParrotState.SHUTDOWN) be after config.recordProcessor.shutdown()? (Maybe not if it is strictly indicating the parrot system's state and excluding the user supplied load test logic.)

My apologies for this not being a pull request, but I'm completely new to Iago, Scala, and Maven; I've yet to figure out how to use a local custom version of Iago from my own test project.

Thanks in advance,

Julian

— Reply to this email directly or view it on GitHubhttps://github.com/twitter/iago/issues/19 .

[julianrendell]

Thanks @WamBamBoozle - I'll fork & (learn how to) do a pull request :-)

I would like to test it locally first though; can you give me a pointer to how to build it locally and then use that build from a separate maven based project? Eg I've created https://github.com/julianrendell/iago-examples and can easily test the patch if I can figure out how to get it to use my local build.

(It should look familiar- I've just copied one of the Iago examples and blindly changed things until they appear to be working; slowly adding a few extra bits and pieces as I read pieces of the main code.)

Thanks in advance,

Julian

[julianrendell]

Answering my own question- looks like maven install will do what I'm looking for.

[julianrendell]

Works for me :+1: