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

_find_missing_instances_dir(... isn't working when disabled configs exist, causing sr3 sanity to not work #780

Closed
reidsunderland opened this issue Oct 16, 2023 · 3 comments
Assignees
Labels
bug Something isn't working likely-fixed likely fix is in the repository, success not confirmed yet. Priority 3 - Important worrisome impact... should study carefully

Comments

@reidsunderland
Copy link
Member

When a disabled config is encountered, _find_missing_instances_dir stops because it gets lost in the directory structure. I added comments showing the directory structure at each step and where the missing os.chdir('..') is.

This causes issues with sr3 sanity, and maybe other parts of the code that depend on knowing about missing instances.

sarracenia/sarracenia/sr.py

Lines 512 to 562 in dedb8d2

def _find_missing_instances_dir(self, dir):
""" find processes which are no longer running, based on pidfiles in state, and procs.
"""
missing = []
if not os.path.isdir(dir):
return
os.chdir(dir)
for c in self.components:
if os.path.isdir(c):
if c not in self.configs: continue
os.chdir(c)
for cfg in os.listdir():
if cfg[0] == '.': continue
if cfg not in self.configs[c]: continue
if os.path.isdir(cfg):
os.chdir(cfg)
if os.path.exists("disabled"): # double check, if disabled should ignore state.
continue
for filename in os.listdir():
# look at pid files, find ones where process is missing.
if filename[-4:] == '.pid':
i = int(filename[-6:-4])
if i != 0:
p = pathlib.Path(filename)
if sys.version_info[0] > 3 or sys.version_info[
1] > 4:
t = p.read_text().strip()
else:
with p.open() as f:
t = f.read().strip()
if t.isdigit():
pid = int(t)
if pid not in self.procs:
missing.append([c, cfg, i])
else:
missing.append([c, cfg, i])
if ( len(self.states[c][cfg]['instance_pids']) > 0 ) or ( len(missing) > 0 ) :
# look for instances that should be running, but no pid file exists.
for i in range(1, int(self.configs[c][cfg]['instances'])+1 ):
if not i in self.states[c][cfg]['instance_pids']:
if i not in self.procs:
if i != 0:
missing.append([c,cfg,i])
os.chdir('..')
os.chdir('..')
self.missing.extend(missing)

def _find_missing_instances_dir(self, dir):
        """ find processes which are no longer running, based on pidfiles in state, and procs.
        """
        missing = []
        if not os.path.isdir(dir):
            return
        os.chdir(dir)
        # in .../.cache/sr3/
        for c in self.components:
            if os.path.isdir(c):
                if c not in self.configs: continue
                os.chdir(c)
                # in .../.cache/sr3/c
                for cfg in os.listdir():
                    if cfg[0] == '.': continue
                    
                    if cfg not in self.configs[c]: continue

                    if os.path.isdir(cfg):
                        os.chdir(cfg)

                        # in .../.cache/sr3/c/cfg

                        if os.path.exists("disabled"): # double check, if disabled should ignore state.
                            # need to os.chdir('..') here
                            continue

                        for filename in os.listdir():
                            # look at pid files, find ones where process is missing.
                            if filename[-4:] == '.pid':
                                i = int(filename[-6:-4])
                                if i != 0:
                                    p = pathlib.Path(filename)
                                    if sys.version_info[0] > 3 or sys.version_info[
                                            1] > 4:
                                        t = p.read_text().strip()
                                    else:
                                        with p.open() as f:
                                            t = f.read().strip()
                                    if t.isdigit():
                                        pid = int(t)
                                        if pid not in self.procs:
                                            missing.append([c, cfg, i])
                                    else:
                                        missing.append([c, cfg, i])
                        if ( len(self.states[c][cfg]['instance_pids']) > 0 ) or ( len(missing) > 0 ) :
                            # look for instances that should be running, but no pid file exists.
                            for i in range(1, int(self.configs[c][cfg]['instances'])+1 ):
                                if not i in self.states[c][cfg]['instance_pids']:
                                    if i not in self.procs:
                                        if i != 0:
                                            missing.append([c,cfg,i])
                        os.chdir('..')
                os.chdir('..')

        self.missing.extend(missing)
@petersilva
Copy link
Contributor

nice catch!

I wonder if the chdir( '..' ) weakness should be addressed, as long as you are here anyways.
like the outer loop... there is already a "dir" can cd to instead of '..' just make a variable for each inner loop, I guess.

@petersilva
Copy link
Contributor

(The chdir('..') weakness == if some dirs are symlinked, then chdir can lead somewhere unexpected... )

@reidsunderland
Copy link
Member Author

i added a new commit that gets rid of '..' in _find_missing_instances_dir : 0b004de

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working likely-fixed likely fix is in the repository, success not confirmed yet. Priority 3 - Important worrisome impact... should study carefully
Projects
None yet
Development

No branches or pull requests

2 participants