Skip to content

JVM Self Destruct#127

Merged
rzezeski merged 2 commits intomasterfrom
bch-solr-monitor
Jun 27, 2013
Merged

JVM Self Destruct#127
rzezeski merged 2 commits intomasterfrom
bch-solr-monitor

Conversation

@hazen
Copy link
Copy Markdown

@hazen hazen commented Jun 13, 2013

This PR closes Issue #124

@ghost ghost assigned hazen Jun 13, 2013
@coderoshi
Copy link
Copy Markdown
Contributor

YES! It works as advertised, at least on my OSX. We need to figure out how to write a test for this, and try it on other machines. But it certainly killed what it needed to kill for me.

@rzezeski
Copy link
Copy Markdown
Contributor

Nice, going to test this today. But my first review comment is about the commit msg. Mind doing it in subject + body style (http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html). I know it's a bit of a contrived thing but various git tools seem to play better with commit msgs that follow that style.

@rzezeski
Copy link
Copy Markdown
Contributor

Why is the Monitor class under a different path than the other custom Java code? I.e. priv/java_monitor. I would like to see it all under priv/java and in the yokozuna.jar.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The copying of this script should be in the grab-solr.sh script. This script is solely for building the yokozuna.jar. The grab-solr.sh is what builds the priv/solr directory.

@hazen
Copy link
Copy Markdown
Author

hazen commented Jun 14, 2013

Nice to see some git commit caremadness. My "favorite" is when people write things like "Checked in code changes." I'll look at that website, @rzezeski .

So I initially put all the Java in the same jar, but it needs to be in a different place for Jetty, per my discussions with @coderoshi . It would be cleaner to have a single, duplicated jar, I guess, if that does not confuse Java. One is for Jetty and one for Solr (as I understand it.)

@hazen
Copy link
Copy Markdown
Author

hazen commented Jun 14, 2013

We seem to have 2 methods of distribution: git build and binary download. We would not need the copy if I could have updated the Amazon tarball. I'm happy with killing the patch file, if you still think it's a good idea.

@rzezeski
Copy link
Copy Markdown
Contributor

We seem to have 2 methods of distribution: git build and binary download. We would not need the copy if I could have updated the Amazon tarball. I'm happy with killing the patch file, if you still think it's a good idea.

Not sure I completely follow. Everything is built on top of the custom binary build. The grab-solr.sh grabs that custom binary and from that builds the solr dir which is used in the Yokozuna release and everything based on it (e.g. the AMI). The patch file is redundant.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Line comments start with double %.

@rzezeski
Copy link
Copy Markdown
Contributor

Would like to have @jaredmorrow give a look at monitor.sh to make sure he doesn't see anything that would cause issues on certain platforms.

@jaredmorrow
Copy link
Copy Markdown
Contributor

monitor.sh is fine I looked at it and just checked out on my most picky platform. The other .sh files though are bash scripts so you'd obviously need bash installed for those to work.

@rzezeski
Copy link
Copy Markdown
Contributor

@jaredmorrow thx for the quick turn around. That's a good point about the other scripts. They are used during the actual building of Yokozuna so once we start migrating into master I suppose it will be important to update those if we want them to build w/o requiring installing bash? Not sure how much I rely on bash-only behavior or non-standard arguments to various commands. These build-time issues can be dealt with once that integration starts happening (hopefully in July).

@jaredmorrow
Copy link
Copy Markdown
Contributor

All of my builders have bash installed. So if they are only build time, I think we are fine.

Brett Hazen and others added 2 commits June 26, 2013 20:10
Jetty will launch a thread to periodically make sure YZ is
still running; if not kill itself
* Fix compile path for `java_monitor` code.

* Use repeating task at fixed rate (`readyForSuicide` was only running once).

* Need to use static class in static context.

* Add some additional output in test.
@rzezeski
Copy link
Copy Markdown
Contributor

Hey Brett, I think you re-ran the verify script without blowing away your verify directory first. You need to do this when you change files involving the solr dir because that stuff is not symlinked like the beam files. Thus you were just testing your old code.

Had to make a few changes to get it to work but it passes for me now.

rzezeski added a commit that referenced this pull request Jun 27, 2013
@rzezeski rzezeski merged commit ef315c5 into master Jun 27, 2013
@hazen
Copy link
Copy Markdown
Author

hazen commented Jun 27, 2013

Bummer. I was hoping to have tested the right thing. Guess I was a bit too hasty. I need to nuke solr to get that part to rebuild properly, right? I wonder if we should include that in the "make clean" target?

@rzezeski
Copy link
Copy Markdown
Contributor

Not just the solr dir. The rt and riak_yz in the verify dir also need be nuked because the devrels need to be rebuilt.

Not sure we want to delete solr during clean. That will cause the tar to be downloaded (although perhaps that should be changed, only download if the md5 doesn't match).

Perhaps a dist-clean or something. Or add a solr-clean and clean-all?

@ghost ghost assigned engelsanchez Oct 2, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants