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.

Sign up for GitHub

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

Jump to bottom

[tune] fix Tensorboard file descriptor leak #12425

Merged
merged 9 commits into from
Dec 3, 2020
Merged

[tune] fix Tensorboard file descriptor leak #12425

merged 9 commits into from
Dec 3, 2020

Conversation

richardliaw
Copy link
Contributor

@richardliaw richardliaw commented Nov 25, 2020 •
edited
Loading

Why are these changes needed?

We have a tensorboard file-descriptor leak.

 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00005_5_a=-1_2020-11-25_12-55-11/events.out.tfevents.1606337713.Richards-MBP.attlocal.net', fd=92),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00007_7_a=-1_2020-11-25_12-55-11/events.out.tfevents.1606337713.Richards-MBP.attlocal.net', fd=94),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00003_3_a=-1_2020-11-25_12-55-10/events.out.tfevents.1606337714.Richards-MBP.attlocal.net', fd=96),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00000_0_a=1_2020-11-25_12-55-10/events.out.tfevents.1606337714.Richards-MBP.attlocal.net', fd=97),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00001_1_a=-1_2020-11-25_12-55-10/events.out.tfevents.1606337714.Richards-MBP.attlocal.net', fd=98),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00002_2_a=1_2020-11-25_12-55-10/events.out.tfevents.1606337715.Richards-MBP.attlocal.net', fd=99),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00003_3_a=-1_2020-11-25_12-55-10/events.out.tfevents.1606337715.Richards-MBP.attlocal.net', fd=100),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00004_4_a=1_2020-11-25_12-55-11/events.out.tfevents.1606337715.Richards-MBP.attlocal.net', fd=101),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00005_5_a=-1_2020-11-25_12-55-11/events.out.tfevents.1606337715.Richards-MBP.attlocal.net', fd=102),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00006_6_a=1_2020-11-25_12-55-11/events.out.tfevents.1606337715.Richards-MBP.attlocal.net', fd=103),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00007_7_a=-1_2020-11-25_12-55-11/events.out.tfevents.1606337715.Richards-MBP.attlocal.net', fd=104),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00000_0_a=1_2020-11-25_12-55-10/events.out.tfevents.1606337716.Richards-MBP.attlocal.net', fd=105),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00001_1_a=-1_2020-11-25_12-55-10/events.out.tfevents.1606337716.Richards-MBP.attlocal.net', fd=106),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00002_2_a=1_2020-11-25_12-55-10/events.out.tfevents.1606337716.Richards-MBP.attlocal.net', fd=107),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00003_3_a=-1_2020-11-25_12-55-10/events.out.tfevents.1606337716.Richards-MBP.attlocal.net', fd=108),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00004_4_a=1_2020-11-25_12-55-11/events.out.tfevents.1606337716.Richards-MBP.attlocal.net', fd=109),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00005_5_a=-1_2020-11-25_12-55-11/events.out.tfevents.1606337717.Richards-MBP.attlocal.net', fd=110),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00006_6_a=1_2020-11-25_12-55-11/events.out.tfevents.1606337717.Richards-MBP.attlocal.net', fd=111),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00007_7_a=-1_2020-11-25_12-55-11/events.out.tfevents.1606337717.Richards-MBP.attlocal.net', fd=112),
 popenfile(path='/Users/rliaw/ray_results/ray_demo/MyTrainable_81eaf_00000_0_a=1_2020-11-25_12-55-10/events.out.tfevents.1606337717.Richards-MBP.attlocal.net', fd=113)]

when running:


        pbt = PopulationBasedTraining(
            time_attr="training_iteration",
            metric="metric",
            mode="max",
            perturbation_interval=1,
            quantile_fraction=0.5,
            hyperparam_mutations={"b": [-1]},
        )

        tune.run(
            MyTrainable,
            name="ray_demo",
            scheduler=pbt,
            stop={"training_iteration": 20},
            num_samples=8,
            checkpoint_freq=1,
            keep_checkpoints_num=1,
            verbose=False,
            fail_fast=True,
            config={"a": tune.sample_from(lambda _: param_a())},
            trial_executor=CustomExecutor(
                queue_trials=False, reuse_actors=False),
        )

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@richardliaw
add-gc-fix
0389b39 
Signed-off-by: Richard Liaw <rliaw@berkeley.edu>
@richardliaw
fd
f8a52c9 
Signed-off-by: Richard Liaw <rliaw@berkeley.edu>
@richardliaw
fix
34d8034 
Signed-off-by: Richard Liaw <rliaw@berkeley.edu>
@richardliaw
p
10088e7 
Signed-off-by: Richard Liaw <rliaw@berkeley.edu>
@richardliaw
Merge branch 'master' into file-descriptor-tune
eb434f1
@richardliaw
fix
2cef946 
Signed-off-by: Richard Liaw <rliaw@berkeley.edu>
@richardliaw
fix-restores
3e56988 
Signed-off-by: Richard Liaw <rliaw@berkeley.edu>
@richardliaw richardliaw marked this pull request as ready for review December 3, 2020 00:54
amogkam
amogkam approved these changes Dec 3, 2020
View reviewed changes
Copy link
Contributor

@amogkam amogkam left a comment

Choose a reason for hiding this comment

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

LGTM. Theres just some extra comments to be removed.

python/ray/tune/tests/test_trial_scheduler_pbt.py Outdated
def testFileFree(self):
class MyTrainable(Trainable):
def setup(self, config):
# Make sure this is large enough so ray uses object store
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great catch

python/ray/tune/tests/test_trial_scheduler_pbt.py Outdated
self.verbose = verbose

def on_trial_result(self, *args, **kwargs):
# assert len(ray.objects()) <= 10
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great catch

@richardliaw
fix
fdd48ca 
Signed-off-by: Richard Liaw <rliaw@berkeley.edu>
@richardliaw
extra-psutil
2c4590c 
Signed-off-by: Richard Liaw <rliaw@berkeley.edu>
@richardliaw
Copy link
Contributor Author

@amogkam thanks for the fast review!

@richardliaw richardliaw mentioned this pull request Dec 3, 2020
Merged
6 tasks
@richardliaw richardliaw added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Dec 3, 2020
@richardliaw richardliaw changed the title [tune] file descriptor leak [tune] fix Tensorboard file descriptor leak Dec 3, 2020
@richardliaw richardliaw merged commit 7c58a85 into ray-project:master Dec 3, 2020
@richardliaw richardliaw deleted the file-descriptor-tune branch December 3, 2020 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amogkam amogkam amogkam approved these changes

@krfricke krfricke krfricke approved these changes

Assignees
No one assigned
Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@richardliaw @amogkam @krfricke