nebius/SWE-rebench-V2-PRs huggingface dataset:
Repository: zalando/patroni
Pull request #1770 (2020-11-26 13:16:58)

add missing shutdown_request
This patch fixes the error handling of cases where there are runtime errors in `socketserver`.
For example, when creating a new thread (to handle a request) fails.

`get_request` handles ssl connections by replacing the new client socket by a tuple containing `(server_socket, new_client_socket)` in order to later deal with handshakes in `process_request_thread`

During the processing of a request, the socketserver `BaseServer` calls `handle_request`, calling the `_handle_request_noblock`, which is calling the following functions (https://github.com/python/cpython/blob/3.8/Lib/socketserver.py#L303):

```
request, client_addr = get_request()
verify_request(request, client_address):
process_request(request, client_address)
handle_error(request, client_address)
shutdown_request(request)
```

- `get_request` is overloaded in patroni and returns `request` as a tuple in case of ssl calls
- `verify_request` defaults to `return True` and should be fixed if used but is fine in this case
- `process_request` just calls `process_request_thread` (which is overloaded in patroni and handles tuple-style requests)
- `handle_error` is overloaded in patroni and handles tuple-style requests)
- but `shutdown_request` is not overloaded and thus missing support for tuple-style requests

This patch adds support for tuple-style requests in patroni api

FxKu: :+1: 
CyberDem0n: :+1:

Patch:
diff --git a/patroni/api.py b/patroni/api.py
index c596e0ec..d24bae96 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -664,6 +664,11 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread):
             newsock = (sock, newsock)
         return newsock, addr
 
+    def shutdown_request(self, request):
+        if isinstance(request, tuple):
+            _, request = request  # SSLSocket
+        return super(RestApiServer, self).shutdown_request(request)
+
     def reload_config(self, config):
         if 'listen' not in config:  # changing config in runtime
             raise ValueError('Can not find "restapi.listen" config')


Test patch:
diff --git a/tests/test_api.py b/tests/test_api.py
index 19c76859..a382fa6f 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -488,3 +488,16 @@ class TestRestApiServer(unittest.TestCase):
         mock_accept.return_value = (newsock, '2')
         self.srv.socket = Mock()
         self.assertEqual(self.srv.get_request(), ((self.srv.socket, newsock), '2'))
+
+    @patch.object(MockRestApiServer, 'process_request', Mock(side_effect=RuntimeError))
+    def test_process_request_error(self):
+        mock_address = ('127.0.0.1', 55555)
+        mock_socket = Mock()
+        mock_ssl_socket = (Mock(), Mock())
+        for mock_request in (mock_socket, mock_ssl_socket):
+            with patch.object(
+                MockRestApiServer,
+                'get_request',
+                Mock(return_value=(mock_request, mock_address))
+            ):
+                self.srv._handle_request_noblock()
Repository: zalando/patroni
Pull request #1933 (2021-05-10 05:26:22)

Fix bug with unix socket connections
When the unix_socket_directories is not known Patroni was immediately going back to tcp connection via the localhost.

The bug was introduced in https://github.com/zalando/patroni/pull/1865

Jan-M: .+1:
Jan-M: :+1:
CyberDem0n: :+1:

Patch:
diff --git a/patroni/postgresql/config.py b/patroni/postgresql/config.py
index 234b2523..d0e5fc5b 100644
--- a/patroni/postgresql/config.py
+++ b/patroni/postgresql/config.py
@@ -879,13 +879,13 @@ class ConfigHandler(object):
         tcp_local_address = self._get_tcp_local_address()
         netloc = self._config.get('connect_address') or tcp_local_address + ':' + port
 
+        unix_local_address = {'port': port}
         unix_socket_directories = self._server_parameters.get('unix_socket_directories')
-        # fallback to tcp if unix_socket_directories is set, but there are no sutable values
-        unix_local_address = unix_socket_directories and\
-            self._get_unix_local_address(unix_socket_directories) or tcp_local_address
+        if unix_socket_directories is not None:
+            # fallback to tcp if unix_socket_directories is set, but there are no sutable values
+            unix_local_address['host'] = self._get_unix_local_address(unix_socket_directories) or tcp_local_address
 
         tcp_local_address = {'host': tcp_local_address, 'port': port}
-        unix_local_address = {'host': unix_local_address, 'port': port}
 
         self._local_address = unix_local_address if self._config.get('use_unix_socket') else tcp_local_address
         self.local_replication_address = unix_local_address\


Test patch:
diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index a13dd73a..42d01e6b 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -492,6 +492,14 @@ class TestPostgresql(BaseTestPostgresql):
         self.p.reload_config(config)
         self.p.config.resolve_connection_addresses()
 
+    def test_resolve_connection_addresses(self):
+        self.p.config._config['use_unix_socket'] = self.p.config._config['use_unix_socket_repl'] = True
+        self.p.config.resolve_connection_addresses()
+        self.assertEqual(self.p.config.local_replication_address, {'host': '/tmp', 'port': '5432'})
+        self.p.config._server_parameters.pop('unix_socket_directories')
+        self.p.config.resolve_connection_addresses()
+        self.assertEqual(self.p.config._local_address, {'port': '5432'})
+
     @patch.object(Postgresql, '_version_file_exists', Mock(return_value=True))
     def test_get_major_version(self):
         with patch.object(builtins, 'open', mock_open(read_data='9.4')):
Repository: zalando/patroni
Pull request #1946 (2021-06-02 06:09:59)

Copy the logical slot over if advance failed due to the missing WAL
It could happen that the replica for some reason is missing the WAL file required by the replication slot.
The nature of this phenomenon is a bit unclear, it might be that the WAL was recycled short before we copied the slot file, but, we still need a solution to this problem. If the `pg_replication_slot_advance()` fails with the `UndefinedFile` exception (requested WAL segment pg_wal/... has already been removed), the logical slot on the replica must be recreated.

sdudoladov: :+1: 
CyberDem0n: :+1:

Patch:
diff --git a/patroni/postgresql/slots.py b/patroni/postgresql/slots.py
index cee319d9..43b7e30c 100644
--- a/patroni/postgresql/slots.py
+++ b/patroni/postgresql/slots.py
@@ -5,6 +5,7 @@ import shutil
 
 from collections import defaultdict
 from contextlib import contextmanager
+from psycopg2.errors import UndefinedFile
 
 from .connection import get_connection_cursor
 from .misc import format_lsn
@@ -198,7 +199,9 @@ class SlotsHandler(object):
                         cur.execute("SELECT pg_catalog.pg_replication_slot_advance(%s, %s)",
                                     (name, format_lsn(int(cluster.slots[name]))))
                     except Exception as e:
-                        logger.exception("Failed to advance logical replication slot '%s': %r", name, e)
+                        logger.error("Failed to advance logical replication slot '%s': %r", name, e)
+                        if isinstance(e, UndefinedFile):
+                            create_slots.append(name)
                         self._schedule_load_slots = True
         return create_slots
 
@@ -280,6 +283,8 @@ class SlotsHandler(object):
                     f.write(value['data'])
                     f.flush()
                     os.fsync(f.fileno())
+                if os.path.exists(slot_dir):
+                    shutil.rmtree(slot_dir)
                 os.rename(slot_tmp_dir, slot_dir)
                 fsync_dir(slot_dir)
                 self._unready_logical_slots.add(name)


Test patch:
diff --git a/tests/test_slots.py b/tests/test_slots.py
index e17c4779..8c281ce7 100644
--- a/tests/test_slots.py
+++ b/tests/test_slots.py
@@ -86,8 +86,8 @@ class TestSlotsHandler(BaseTestPostgresql):
                           [self.me, self.other, self.leadermem], None, None, None, {'ls': 12346})
         self.assertEqual(self.s.sync_replication_slots(cluster, False), [])
         self.s._schedule_load_slots = False
-        with patch.object(MockCursor, 'execute', Mock(side_effect=psycopg2.OperationalError)):
-            self.assertEqual(self.s.sync_replication_slots(cluster, False), [])
+        with patch.object(MockCursor, 'execute', Mock(side_effect=psycopg2.errors.UndefinedFile)):
+            self.assertEqual(self.s.sync_replication_slots(cluster, False), ['ls'])
         cluster.slots['ls'] = 'a'
         self.assertEqual(self.s.sync_replication_slots(cluster, False), [])
         with patch.object(MockCursor, 'rowcount', PropertyMock(return_value=1), create=True):
Repository: zalando/patroni
Pull request #2221 (2022-02-17 11:12:54)

Fix bug in divergence timeline check
Patroni was falsely assuming that timelines have diverged.
For pg_rewind it didn't create any problem, but if pg_rewind is not allowed and the `remove_data_directory_on_diverged_timelines` is set, it resulted in reinitializing the former leader.

Close https://github.com/zalando/patroni/issues/2220

CyberDem0n: :+1:

Patch:
diff --git a/patroni/postgresql/rewind.py b/patroni/postgresql/rewind.py
index b30d739f..ba6e4532 100644
--- a/patroni/postgresql/rewind.py
+++ b/patroni/postgresql/rewind.py
@@ -218,6 +218,7 @@ class Rewind(object):
                         need_rewind = True
                     else:
                         need_rewind = switchpoint != self._get_checkpoint_end(local_timeline, local_lsn)
+                    break
                 elif parent_timeline > local_timeline:
                     need_rewind = True
                     break


Test patch:
diff --git a/tests/test_rewind.py b/tests/test_rewind.py
index 19e7e6ad..54fdae91 100644
--- a/tests/test_rewind.py
+++ b/tests/test_rewind.py
@@ -141,7 +141,10 @@ class TestRewind(BaseTestPostgresql):
         self.leader = self.leader.member
         self.assertFalse(self.r.rewind_or_reinitialize_needed_and_possible(self.leader))
         mock_check_leader_is_not_in_recovery.return_value = True
-        self.assertTrue(self.r.rewind_or_reinitialize_needed_and_possible(self.leader))
+        self.assertFalse(self.r.rewind_or_reinitialize_needed_and_possible(self.leader))
+        self.r.trigger_check_diverged_lsn()
+        with patch.object(MockCursor, 'fetchone', Mock(side_effect=[('', 3, '0/0'), ('', b'4\t0/40159C0\tn\n')])):
+            self.assertTrue(self.r.rewind_or_reinitialize_needed_and_possible(self.leader))
         self.r.reset_state()
         self.r.trigger_check_diverged_lsn()
         with patch('patroni.psycopg.connect', Mock(side_effect=Exception)):
Repository: zalando/patroni
Pull request #2468 (2022-11-30 11:08:55)

Fix annoying exceptions on ssl socket shutdown
The HAProxy is closing connections as soon as it got the HTTP Status code leaving no time for Patroni to properly shutdown SSL connection.

Close https://github.com/zalando/patroni/issues/2466

coveralls: ## Pull Request Test Coverage Report for [Build 3582730071](https://coveralls.io/builds/54694885)

* **0** of **4**   **(100.0%)**  changed or added relevant lines in **1** file are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at ?**%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/54694885/badge)](https://coveralls.io/builds/54694885) |
| :-- | --: |
| Change from base [Build 3565006952](https://coveralls.io/builds/54614239): |  0% |
| Covered Lines: |  |
| Relevant Lines: | 0 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/patroni/api.py b/patroni/api.py
index 657744fd..47f4ce9b 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -845,7 +845,10 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread):
 
     def shutdown_request(self, request):
         if hasattr(request, 'context'):  # SSLSocket
-            request.unwrap()
+            try:
+                request.unwrap()
+            except Exception as e:
+                logger.debug('Failed to shutdown SSL connection: %r', e)
         super(RestApiServer, self).shutdown_request(request)
 
     def get_certificate_serial_number(self):


Test patch:
diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml
index 86bec798..1c261f87 100644
--- a/.github/workflows/tests.yaml
+++ b/.github/workflows/tests.yaml
@@ -10,7 +10,7 @@ on:
 
 jobs:
   unit:
-    runs-on: ${{ matrix.os }}-latest
+    runs-on: ${{ fromJson('{"ubuntu":"ubuntu-20.04","windows":"windows-latest","macos":"macos-latest"}')[matrix.os] }}
     strategy:
       fail-fast: false
       matrix:
@@ -89,7 +89,7 @@ jobs:
       run: python -m coveralls --service=github
 
   behave:
-    runs-on: ${{ matrix.os }}-latest
+    runs-on: ${{ fromJson('{"ubuntu":"ubuntu-20.04","windows":"windows-latest","macos":"macos-latest"}')[matrix.os] }}
     env:
       DCS: ${{ matrix.dcs }}
       ETCDVERSION: 3.3.13
diff --git a/tests/test_api.py b/tests/test_api.py
index a1db15f3..37228980 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -601,8 +601,11 @@ class TestRestApiServer(unittest.TestCase):
         self.srv.process_request_thread(Mock(), '2')
 
     @patch.object(MockRestApiServer, 'process_request', Mock(side_effect=RuntimeError))
-    @patch.object(MockRestApiServer, 'get_request', Mock(return_value=(Mock(), ('127.0.0.1', 55555))))
-    def test_process_request_error(self):
+    @patch.object(MockRestApiServer, 'get_request')
+    def test_process_request_error(self, mock_get_request):
+        mock_request = Mock()
+        mock_request.unwrap.side_effect = Exception
+        mock_get_request.return_value = (mock_request, ('127.0.0.1', 55555))
         self.srv._handle_request_noblock()
 
     @patch('ssl._ssl._test_decode_cert', Mock())