Skip to content

Using lock files for port reservation #247

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
Open
dmitry-lipetsk opened this issue May 4, 2025 · 2 comments · May be fixed by #255
Open

Using lock files for port reservation #247

dmitry-lipetsk opened this issue May 4, 2025 · 2 comments · May be fixed by #255
Assignees
@dmitry-lipetsk
Labels
enhancement

Comments

@dmitry-lipetsk
Copy link
Collaborator

dmitry-lipetsk commented May 4, 2025 •
edited
Loading

Testgres must use lock files to prevent a reservation of the same port by multiple parallel workers.

The current implementation allows the reservation of one port number by multiple parallel workers (on pytest, for eaxample).

It creates the real situation when two test uses one port and do not know about it. Example:

  • One test allocates the port for PG and get it.
  • Another test allocates the same port for Proxima. Proxima does not work but hosted PG is working!

As result, the second test may do not know that it works with wrong port.

Usage of lock files (in the one directory) must help solve this problem.

What are problems?

  1. Testgres may work with "remote" instance of Postgres. So Lock directory must to be on this "remote" host.

Remote host may do not has a direct access to our host and cannot verify that our process directly though PID as TAP tests.

As variant - we can do it through TCP/IP connection. I do not plan to implement it right now but this thing is great task for guys who want to join us )

  1. Different locations of lock files directory

We have to provide the way to explicit definition of lock files directory path.
@dmitry-lipetsk
Copy link
Collaborator Author

This feature closes #235

This was referenced May 4, 2025
Merged
Merged
Closed
dmitry-lipetsk added a commit to dmitry-lipetsk/testgres that referenced this issue May 4, 2025
@dmitry-lipetsk
[Refactoring] Default port manager functions now use PortManager__Gen…
d5bcee6 
…eric and LocalOperations

This patch deletes a duplication of port manager code.

Now utils.reserve_port and utils.release_port works through _old_port_manager - it is a global instance of PortManager__Generic that uses a global instance of LocalOperations.

This commit is a part of work for postgrespro#247.
@dmitry-lipetsk dmitry-lipetsk mentioned this issue May 4, 2025
Merged
dmitry-lipetsk added a commit that referenced this issue May 5, 2025
@dmitry-lipetsk
[Refactoring] Default port manager functions now use PortManager__Gen…
a683c65 
…eric and LocalOperations (#251)

* [Refactoring] Default port manager functions now use PortManager__Generic and LocalOperations

This patch deletes a duplication of port manager code.

Now utils.reserve_port and utils.release_port works through _old_port_manager - it is a global instance of PortManager__Generic that uses a global instance of LocalOperations.

This commit is a part of work for #247.

* [BUG FIX] PortManager__ThisHost::__new__ had MT-problem

After MT-lock we must to check __class__.sm_single_instance again.

Refactoring
 - PortManager__ThisHost::__new__ is replaced with an explicit PortManager__ThisHost::get_single_instance()
 - PortManager__ThisHost::__init__ is deleted
@dmitry-lipetsk dmitry-lipetsk mentioned this issue May 5, 2025
Merged
@dmitry-lipetsk dmitry-lipetsk self-assigned this May 5, 2025
dmitry-lipetsk added a commit to dmitry-lipetsk/testgres that referenced this issue May 5, 2025
@dmitry-lipetsk
[postgrespro#247] PortManager__Generic uses lock-files for reserved p…
0a9b431 
…orts
dmitry-lipetsk added a commit to dmitry-lipetsk/testgres that referenced this issue May 6, 2025
@dmitry-lipetsk
[postgrespro#247] PortManager__Generic uses lock-dirs
9f13a36 
Alpine has a problem with "mv -n" operation - it is not an atomic.

So we will use lock-dirs instead lock-files.
dmitry-lipetsk added a commit to dmitry-lipetsk/testgres that referenced this issue May 6, 2025
@dmitry-lipetsk
New OsOperations methods: makedir, rmdir
2d5ccca 
Signatures:
 def makedir(self, path: str)
 def rmdir(self, path: str)

It is a part of work for postgrespro#247.
@dmitry-lipetsk dmitry-lipetsk mentioned this issue May 6, 2025
Merged
dmitry-lipetsk added a commit that referenced this issue May 6, 2025
@dmitry-lipetsk
New OsOperations methods: makedir, rmdir (#253)
a0a8506 
Signatures:
 def makedir(self, path: str)
 def rmdir(self, path: str)

It is a part of work for #247.
dmitry-lipetsk added a commit to dmitry-lipetsk/testgres that referenced this issue May 6, 2025
@dmitry-lipetsk
[postgrespro#247] PortManager__Generic uses lock-dirs for reserved ports
99e645e
@dmitry-lipetsk dmitry-lipetsk linked a pull request May 6, 2025 that will close this issue
Open
@dmitry-lipetsk
Copy link
Collaborator Author

dmitry-lipetsk commented May 6, 2025 •
edited
Loading

The final implementation is using lock-directories, not lock-files.

It resolves a problem of "remote" operations.

Alpine Linux (in CI) does not have an atomic operations "mv -n" and as result two workers may create the same lock object simultaneously.

mkdir does not has the such problem.

Home folder of lock-dirs is "/tmp/testgres/ports"

@dmitry-lipetsk dmitry-lipetsk mentioned this issue May 11, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dmitry-lipetsk dmitry-lipetsk

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[#247] PortManager__Generic uses lock-dirs for reserved ports dmitry-lipetsk/testgres
1 participant
@dmitry-lipetsk