Skip to content

Update pool.py - #828

Open
glyg wants to merge 1 commit into
irods:mainfrom
glyg:patch-1
Open

Update pool.py#828
glyg wants to merge 1 commit into
irods:mainfrom
glyg:patch-1

Conversation

@glyg

@glyg glyg commented Aug 24, 2026

Copy link
Copy Markdown

Use if instead of a try / except loop to avoid circular call, see #827

Use if instead of a try / except loop to avoid circular call
@korydraughn
korydraughn requested a review from d-w-moore August 24, 2026 12:41

@d-w-moore d-w-moore left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Seems fine. Can run a full test suite later today perhaps, which could be useful if I knew how to invoke #827. Not knowing how one might reproduce the original bug leaves me wondering how to write a good test.

@glyg

glyg commented Aug 25, 2026

Copy link
Copy Markdown
Author

I have trouble reproducing the bug.

Our setup is quite complex, and I manage to produce the bug, but not every time and not at the same time ...
With a populated DB, repeatedly querying irods through the python client leads to the error, but not on a given data object or after a specific number of repetitions.

I am trying to corner the bug better, but for now I can't give you a minimal example

@glyg

glyg commented Aug 26, 2026

Copy link
Copy Markdown
Author

Hi, we tracked it down to too many connection on the DB, which leads to the messy disconnect and death of the server.

image

This does not seem to happen on a pre-prod server, but only in the docker context above, on my machine.

@glyg

glyg commented Aug 26, 2026

Copy link
Copy Markdown
Author

further query to the DB fails

sql: error: connection to server on socket "/var/run/postgresql/.s.PGSQL.5432" failed: FATAL:  sorry, too many clients already

@trel

trel commented Aug 26, 2026

Copy link
Copy Markdown
Member

ah, very good.

so, if we limit the resources artificially, we should be able to produce a test that presents the same behavior.

@glyg

glyg commented Aug 26, 2026

Copy link
Copy Markdown
Author

Yes, we're not seeing it in the prod postgresql VM,

we tracked the Postgresql connections (with : psql -U irods -d ICAT -c "SELECT count(*) FROM pg_stat_activity;" ) and they are 'regulated' in the VM and not in the docker container on my sandbox.

The "too many connections" error ends up killing the irods process though

We updated postgresql to 17 without changing the issue

@glyg

glyg commented Aug 26, 2026

Copy link
Copy Markdown
Author

so, if we limit the resources artificially, we should be able to produce a test that presents the same behavior

I suspect killing the db and trying a request from the python client is enough to reproduce the long traceback at the origin of the issue

@trel

trel commented Aug 26, 2026

Copy link
Copy Markdown
Member

I suspect killing the db and trying a request from the python client is enough to reproduce the long traceback at the origin of the issue

oh, yes, that might do it. definitely not something we've tested before.

@glyg

glyg commented Aug 26, 2026

Copy link
Copy Markdown
Author

It seems that (again, docker only) the idle connections to the postgresql DB are not cleaned and clog the pile ... Who's at fault in the stack though ...

image

@glyg

glyg commented Aug 26, 2026

Copy link
Copy Markdown
Author

So, we appear to have fixed the problem with a strict reuse of the session for each 'agent', so:

class iRODSAgent:
    ...
    def __enter__(self):
        super().__enter__()
        self.session = irods_conn(self.conf)
        return self

    def __exit__(self, exc_type, exc_value, traceback):
        self.session.cleanup()
        super().__exit__(exc_type, exc_value, traceback)

What create zombie session is doing something like:

    # Within the agent class
    def do_smthg(self):
        session = iRODSSession(...)

        session.do_something()

and not

    # Within the agent class
    def do_smthg(self):

        self.session.do_something()

Fewww ^^'

@trel

trel commented Aug 26, 2026

Copy link
Copy Markdown
Member

So I see two things that work?

  • A code change that uses if/else rather than try/except
  • Holding the current code differently

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants