Bump version and write release notes (#560)

and implement missing unit-tests
This commit is contained in:
Alexander Kukushkin
2017-11-10 11:48:50 +01:00
committed by GitHub
parent 2e86fe5991
commit a89a902f4a
4 changed files with 97 additions and 26 deletions
+41
View File
@@ -3,6 +3,47 @@
Release notes
=============
Version 1.3.6
-------------
**Stability improvements**
- Verify process start time when checking if postgres is running. (Ants Aasma)
After a crash that doesn't clean up postmaster.pid there could be a new process with the same pid, resulting in a false positive for is_running(), which will lead to all kinds of bad behavior.
- Shutdown postgresql before bootstrap when we lost data directory (ainlolcat)
When data directory on the master is forcefully removed, postgres process can still stay alive for some time and prevent the replica created in place of that former master from starting or replicating.
The fix makes Patroni cache the postmaster pid and its start time and let it terminate the old postmaster in case it is still running after the corresponding data directory has been removed.
- Perform crash recovery in a single user mode if postgres master dies (Alexander Kukushkin)
It is unsafe to start immediately as a standby and not possible to run ``pg_rewind`` if postgres hasn't been shut down cleanly.
The single user crash recovery only kicks in if ``pg_rewind`` is enabled or there is no master at the moment.
**Consul improvements**
- Make it possible to provide datacenter configuration for Consul (DeathBorn, Alexander)
Before that Patroni was always communicating with datacenter of the host it runs on.
- Always send a token in X-Consul-Token http header (Alexander)
If ``consul.token`` is defined in Patroni configuration, we will always send it in the 'X-Consul-Token' http header.
python-consul module tries to be "consistent" with Consul REST API, which doesn't accept token as a query parameter for `session API <https://www.consul.io/api/session.html>`__, but it still works with 'X-Consul-Token' header.
- Adjust session TTL if supplied value is smaller than the minimum possible (Stas Fomin, Alexander)
It could happen that the TTL provided in the Patroni configuration is smaller than the minimum one supported by Consul. In that case, Consul agent fails to create a new session.
Without a session Patroni cannot create member and leader keys in the Consul KV store, resulting in an unhealthy cluster.
**Other improvements**
- Define custom log format via environment variable ``PATRONI_LOGFORMAT`` (Stas)
Allow disabling timestamps and other similar fields in Patroni logs if they are already added by the system logger (usually when Patroni runs as a service).
Version 1.3.5
-------------
+20 -19
View File
@@ -71,18 +71,18 @@ def null_context():
yield
def _update_postmaster_info(func):
def _update_postmaster_cached_info(func):
def wrapper(self):
ret = func(self)
if ret and 'pid' in ret and 'start_time' in ret:
old_pid = self._postmaster_info.get('pid', 0)
old_start_time = self._postmaster_info.get('start_time', 0)
old_pid = self._postmaster_cached_info.get('pid', 0)
old_start_time = self._postmaster_cached_info.get('start_time', 0)
try:
pmpid = int(ret['pid'])
pmstart = int(ret['start_time'])
if pmpid != old_pid or pmstart != old_start_time: # this check removes repeating messages from logs
self._postmaster_info = {'pid': pmpid, 'start_time': pmstart}
logger.info("Updated postmaster info: %s .", self._postmaster_info)
self._postmaster_cached_info = {'pid': pmpid, 'start_time': pmstart}
logger.info("Updated postmaster info: %s .", self._postmaster_cached_info)
except ValueError:
logger.warning('Cannot update postmaster info with data due garbage in pid file: %s', ret)
return ret
@@ -161,7 +161,7 @@ class Postgresql(object):
self._pg_hba_conf = os.path.join(self._config_dir, 'pg_hba.conf')
self._recovery_conf = os.path.join(self._data_dir, 'recovery.conf')
self._postmaster_pid = os.path.join(self._data_dir, 'postmaster.pid')
self._postmaster_info = {'pid': 0, 'start_time': 0}
self._postmaster_cached_info = {'pid': 0, 'start_time': 0}
self._trigger_file = config.get('recovery_conf', {}).get('trigger_file') or 'promote'
self._trigger_file = os.path.abspath(os.path.join(self._data_dir, self._trigger_file))
@@ -732,7 +732,7 @@ class Postgresql(object):
return self._is_postmaster_pid_running(int_or_none(pidfile.get('pid')),
start_time=int_or_none(pidfile.get('start_time')))
@_update_postmaster_info
@_update_postmaster_cached_info
def read_pid_file(self):
"""Reads and parses postmaster.pid from the data directory
@@ -759,21 +759,21 @@ class Postgresql(object):
def get_pid_with_lost_data_dir(self):
logger.info("Trying to check if process running without directory "
"with cached postmaster info: %s .", self._postmaster_info)
"with cached postmaster info: %s .", self._postmaster_cached_info)
try:
process = psutil.Process(self._postmaster_info['pid'])
process = psutil.Process(self._postmaster_cached_info['pid'])
# check difference instead of values because of rounding issues
if abs(self._postmaster_info["start_time"] - process.create_time()) < 2:
if abs(self._postmaster_cached_info["start_time"] - process.create_time()) < 2:
return process.pid
else:
logger.info("Process with pid %s was started at different time %s .",
process.pid, process.create_time())
except psutil.NoSuchProcess:
logger.info("Cannot find process %s .", self._postmaster_info['pid'])
logger.info("Cannot find process %s .", self._postmaster_cached_info['pid'])
return 0
def clean_postmaster_info(self):
self._postmaster_info = {'pid': 0, 'start_time': 0}
def clean_postmaster_cached_info(self):
self._postmaster_cached_info = {'pid': 0, 'start_time': 0}
logger.info("postmaster info was cleaned.")
@staticmethod
@@ -991,11 +991,12 @@ class Postgresql(object):
def _do_stop(self, mode, block_callbacks, checkpoint, on_safepoint):
if not self.is_running():
pid = self.get_pid_with_lost_data_dir()
if pid > 0:
self.terminate_starting_postmaster(pid)
self.clean_postmaster_info()
return True, True
if self.data_directory_empty() and self._postmaster_cached_info['pid']:
pid = self.get_pid_with_lost_data_dir()
if pid > 0:
self.terminate_starting_postmaster(pid)
self.clean_postmaster_cached_info()
return True, True
if on_safepoint:
on_safepoint()
return True, False
@@ -1021,7 +1022,7 @@ class Postgresql(object):
on_safepoint()
self._wait_for_postmaster_stop(pid)
self.clean_postmaster_info()
self.clean_postmaster_cached_info()
return True, True
+1 -1
View File
@@ -1 +1 @@
__version__ = '1.3.5'
__version__ = '1.3.6'
+35 -6
View File
@@ -277,6 +277,21 @@ class TestPostgresql(unittest.TestCase):
mock_is_running.return_value = False
self.assertTrue(self.p.stop(on_safepoint=mock_callback))
mock_callback.assert_called()
with patch.object(Postgresql, '_is_postmaster_pid_running', Mock(return_value=False)), \
patch.object(Postgresql, 'data_directory_empty', Mock(return_value=True)):
with patch('psutil.Process') as mock_psutil:
self.p._postmaster_cached_info = {'pid': 1, 'start_time': 1}
mock_psutil.return_value.pid = 1
mock_psutil.return_value.create_time.return_value = 1
self.assertTrue(self.p.stop())
self.p._postmaster_cached_info = {'pid': 1, 'start_time': 1}
mock_psutil.return_value.create_time.return_value = 100
self.assertTrue(self.p.stop())
self.p._postmaster_cached_info = {'pid': 1, 'start_time': 1}
mock_psutil.side_effect = psutil.NoSuchProcess('')
self.assertTrue(self.p.stop())
mock_is_running.return_value = True
mock_get_pid.return_value = 0
mock_callback.reset_mock()
@@ -773,13 +788,19 @@ class TestPostgresql(unittest.TestCase):
os.remove(pidfile)
self.assertEquals(self.p.read_pid_file(), {})
@patch.object(Postgresql, '_version_file_exists', Mock(return_value=True))
@patch('os.path.isfile', Mock(return_value=True))
@patch.object(Postgresql, 'read_pid_file')
@patch('psutil.Process')
def test_is_postmaster_pid_running(self, mock_psutil):
mock_proc = Mock()
mock_psutil.return_value = mock_proc
self.assertTrue(self.p._is_postmaster_pid_running(-100))
self.assertFalse(self.p._is_postmaster_pid_running(0))
self.assertFalse(self.p._is_postmaster_pid_running(None))
def test_is_postmaster_pid_running(self, mock_psutil, mock_read_pid_file):
mock_psutil.return_value.create_time.return_value = 1
mock_read_pid_file.return_value = {'pid': -100, 'start_time': 1}
self.assertTrue(self.p.is_running())
with patch('os.getpid', Mock(return_value=100)):
mock_read_pid_file.return_value = {'pid': 100, 'start_time': 1}
self.assertFalse(self.p.is_running())
mock_read_pid_file.return_value = {'pid': 100, 'start_time': 100}
self.assertFalse(self.p.is_running())
def test_pick_sync_standby(self):
cluster = Cluster(True, None, self.leader, 0, [self.me, self.other, self.leadermem], None,
@@ -924,3 +945,11 @@ class TestPostgresql(unittest.TestCase):
@patch.object(Postgresql, 'single_user_mode', Mock(return_value=0))
def test_fix_cluster_state(self):
self.assertTrue(self.p.fix_cluster_state())
def test__update_postmaster_cached_info(self):
with open(os.path.join(self.data_dir, 'postmaster.pid'), 'w') as f:
f.write('1\n\n1\n')
self.p.read_pid_file()
with open(os.path.join(self.data_dir, 'postmaster.pid'), 'w') as f:
f.write('a\n\n1\n')
self.p.read_pid_file()