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

Add broker.pid attribute to broker, Fix broker corner case with scratch-directory-rank #954

Merged
merged 6 commits into from Jan 18, 2017

Conversation

Projects
None yet
5 participants
@chu11
Copy link
Contributor

chu11 commented Jan 14, 2017

Per issue #839

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 14, 2017

Current coverage is 76.04% (diff: 73.33%)

Merging #954 into master will decrease coverage by 0.01%

@@             master       #954   diff @@
==========================================
  Files           149        149          
  Lines         25962      25970     +8   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          19749      19750     +1   
- Misses         6213       6220     +7   
  Partials          0          0          
Diff Coverage File Path
••••••• 73% src/broker/broker.c

Powered by Codecov. Last update 15709a9...ef9fee1

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 14, 2017

Coverage Status

Coverage decreased (-0.01%) to 76.339% when pulling 7d71473 on chu11:issue839 into c55822f on flux-framework:master.

chu11 added some commits Jan 13, 2017

src/broker/broker.c: Use cached value
Use cached pid value instead of calling getpid().

@chu11 chu11 force-pushed the chu11:issue839 branch from 7d71473 to 93a5b60 Jan 17, 2017

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 17, 2017

Coverage Status

Coverage decreased (-0.01%) to 76.343% when pulling 93a5b60 on chu11:issue839 into 15709a9 on flux-framework:master.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Jan 17, 2017

This all looks good to me. Want to throw in a fix for this error message typo (sort of related I guess)?

lt-flux-broker: could not initialize ankdir: No such file or directory
@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Jan 17, 2017

Look great to me too, thanks!

Very minor, but if you are going to do a little more work on this one, perhaps fix the commit message typo in 42ef88e :

cat /proc/$(flux getattr broker-pid)/status

Should be "broker.pid" not "broker-pid" (I think?)

@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Jan 17, 2017

@garlick Ahh good eye. Will fix up.
@grondo Thanks for the catch, I blindly copied from the issue tracker.

src/broker/broker.c: Add "broker.pid" attr
Add new "broker.pid" attribute, which may be useful for debugging,
such as with

cat /proc/$(flux getattr broker.pid)/status

Fixes #839

@chu11 chu11 force-pushed the chu11:issue839 branch from 93a5b60 to e8c25a5 Jan 17, 2017

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 17, 2017

Coverage Status

Coverage decreased (-0.002%) to 76.35% when pulling e8c25a5 on chu11:issue839 into 15709a9 on flux-framework:master.

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Jan 17, 2017

@grondo Thanks for the catch, I blindly copied from the issue tracker.

Oops, sorry about that -- probably my fault then!

chu11 added some commits Jan 14, 2017

src/broker/broker.c: Fix update_pidfile corner case
Store pidfile in 'scratch-directory-rank' attribute and not
'scratch-directory'/RANK.  Broker can fail if user chose to
set the 'scratch-directory-rank' attribute but not the 'scratch-directory'
attribute.
test/basic: Add new attr tests
Add new scratch-directory-rank test
Add broker.pid test
Add broker.pid immutable test

@chu11 chu11 force-pushed the chu11:issue839 branch from e8c25a5 to ef9fee1 Jan 17, 2017

@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Jan 17, 2017

oops, found one other commit message typo. Re-pushed.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 17, 2017

Coverage Status

Coverage decreased (-0.02%) to 76.328% when pulling ef9fee1 on chu11:issue839 into 15709a9 on flux-framework:master.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Jan 18, 2017

Thanks! Merging.

@garlick garlick merged commit 40a6549 into flux-framework:master Jan 18, 2017

2 of 4 checks passed

codecov/patch 73.33% of diff hit (target 76.06%)
Details
codecov/project 76.04% (-0.02%) compared to 15709a9
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.02%) to 76.328%
Details

@grondo grondo referenced this pull request Mar 28, 2017

Closed

0.7.0 Release Notes #1019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.