Skip to content
Snippets Groups Projects
Commit 39e3fc69 authored by Erik Johnston's avatar Erik Johnston
Browse files

Make the state resolution use actual power levels rather than taking them from a Pdu key.

parent b42fe05c
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
synapse/federation/units.py
+ 1
− 0
View file @ 39e3fc69
......@@ -69,6 +69,7 @@ class Pdu(JsonEncodedObject):
"prev_state_id",
"prev_state_origin",
"required_power_level",
"user_id",
]
internal_keys = [
......
synapse/state.py
+ 35
− 11
View file @ 39e3fc69
......@@ -115,6 +115,8 @@ class StateHandler(object):
is_new = yield self._handle_new_state(new_pdu)
logger.debug("is_new: %s %s %s", is_new, new_pdu.pdu_id, new_pdu.origin)
if is_new:
yield self.store.update_current_state(
pdu_id=new_pdu.pdu_id,
......@@ -187,11 +189,12 @@ class StateHandler(object):
# We didn't find a common ancestor. This is probably fine.
pass
result = self._do_conflict_res(
result = yield self._do_conflict_res(
new_branch, current_branch, common_ancestor
)
defer.returnValue(result)
@defer.inlineCallbacks
def _do_conflict_res(self, new_branch, current_branch, common_ancestor):
conflict_res = [
self._do_power_level_conflict_res,
......@@ -200,7 +203,8 @@ class StateHandler(object):
]
for algo in conflict_res:
new_res, curr_res = algo(
new_res, curr_res = yield defer.maybeDeferred(
algo,
new_branch, current_branch, common_ancestor
)
......@@ -211,19 +215,39 @@ class StateHandler(object):
raise Exception("Conflict resolution failed.")
@defer.inlineCallbacks
def _do_power_level_conflict_res(self, new_branch, current_branch,
common_ancestor):
max_power_new = max(
new_branch[:-1] if common_ancestor else new_branch,
key=lambda t: t.power_level
).power_level
new_powers_deferreds = []
for e in new_branch[:-1] if common_ancestor else new_branch:
if hasattr(e, "user_id"):
new_powers_deferreds.append(
self.store.get_power_level(e.context, e.user_id)
)
current_powers_deferreds = []
for e in current_branch[:-1] if common_ancestor else current_branch:
if hasattr(e, "user_id"):
current_powers_deferreds.append(
self.store.get_power_level(e.context, e.user_id)
)
new_powers = yield defer.gatherResults(
new_powers_deferreds,
consumeErrors=True
)
max_power_current = max(
current_branch[:-1] if common_ancestor else current_branch,
key=lambda t: t.power_level
).power_level
current_powers = yield defer.gatherResults(
current_powers_deferreds,
consumeErrors=True
)
max_power_new = max(new_powers)
max_power_current = max(current_powers)
return (max_power_new, max_power_current)
defer.returnValue(
(max_power_new, max_power_current)
)
def _do_chain_length_conflict_res(self, new_branch, current_branch,
common_ancestor):
......
synapse/storage/_base.py
+ 8
− 0
View file @ 39e3fc69
......@@ -17,6 +17,7 @@ import logging
from twisted.internet import defer
from synapse.api.errors import StoreError
from synapse.util.logutils import log_function
import collections
import copy
......@@ -91,6 +92,7 @@ class SQLBaseStore(object):
self._simple_insert_txn, table, values, or_replace=or_replace
)
@log_function
def _simple_insert_txn(self, txn, table, values, or_replace=False):
sql = "%s INTO %s (%s) VALUES(%s)" % (
("INSERT OR REPLACE" if or_replace else "INSERT"),
......@@ -98,6 +100,12 @@ class SQLBaseStore(object):
", ".join(k for k in values),
", ".join("?" for k in values)
)
logger.debug(
"[SQL] %s Args=%s Func=%s",
sql, values.values(),
)
txn.execute(sql, values.values())
return txn.lastrowid
......
synapse/storage/pdu.py
+ 10
− 71
View file @ 39e3fc69
......@@ -17,6 +17,7 @@ from twisted.internet import defer
from ._base import SQLBaseStore, Table, JoinHelper
from synapse.federation.units import Pdu
from synapse.util.logutils import log_function
from collections import namedtuple
......@@ -625,53 +626,6 @@ class StatePduStore(SQLBaseStore):
return result
def get_next_missing_pdu(self, new_pdu):
"""When we get a new state pdu we need to check whether we need to do
any conflict resolution, if we do then we need to check if we need
to go back and request some more state pdus that we haven't seen yet.
Args:
txn
new_pdu
Returns:
PduIdTuple: A pdu that we are missing, or None if we have all the
pdus required to do the conflict resolution.
"""
return self._db_pool.runInteraction(
self._get_next_missing_pdu, new_pdu
)
def _get_next_missing_pdu(self, txn, new_pdu):
logger.debug(
"get_next_missing_pdu %s %s",
new_pdu.pdu_id, new_pdu.origin
)
current = self._get_current_interaction(
txn,
new_pdu.context, new_pdu.pdu_type, new_pdu.state_key
)
if (not current or not current.prev_state_id
or not current.prev_state_origin):
return None
# Oh look, it's a straight clobber, so wooooo almost no-op.
if (new_pdu.prev_state_id == current.pdu_id
and new_pdu.prev_state_origin == current.origin):
return None
enum_branches = self._enumerate_state_branches(txn, new_pdu, current)
for branch, prev_state, state in enum_branches:
if not state:
return PduIdTuple(
prev_state.prev_state_id,
prev_state.prev_state_origin
)
return None
def handle_new_state(self, new_pdu):
"""Actually perform conflict resolution on the new_pdu on the
assumption we have all the pdus required to perform it.
......@@ -755,24 +709,11 @@ class StatePduStore(SQLBaseStore):
return is_current
@classmethod
@log_function
def _enumerate_state_branches(cls, txn, pdu_a, pdu_b):
def _enumerate_state_branches(self, txn, pdu_a, pdu_b):
branch_a = pdu_a
branch_b = pdu_b
get_query = (
"SELECT %(fields)s FROM %(pdus)s as p "
"LEFT JOIN %(state)s as s "
"ON p.pdu_id = s.pdu_id AND p.origin = s.origin "
"WHERE p.pdu_id = ? AND p.origin = ? "
) % {
"fields": _pdu_state_joiner.get_fields(
PdusTable="p", StatePdusTable="s"),
"pdus": PdusTable.table_name,
"state": StatePdusTable.table_name,
}
while True:
if (branch_a.pdu_id == branch_b.pdu_id
and branch_a.origin == branch_b.origin):
......@@ -804,13 +745,12 @@ class StatePduStore(SQLBaseStore):
branch_a.prev_state_origin
)
logger.debug("getting branch_a prev %s", pdu_tuple)
txn.execute(get_query, pdu_tuple)
prev_branch = branch_a
res = txn.fetchone()
branch_a = PduEntry(*res) if res else None
logger.debug("getting branch_a prev %s", pdu_tuple)
branch_a = self._get_pdu_tuple(txn, *pdu_tuple)
if branch_a:
branch_a = Pdu.from_pdu_tuple(branch_a)
logger.debug("branch_a=%s", branch_a)
......@@ -823,14 +763,13 @@ class StatePduStore(SQLBaseStore):
branch_b.prev_state_id,
branch_b.prev_state_origin
)
txn.execute(get_query, pdu_tuple)
logger.debug("getting branch_b prev %s", pdu_tuple)
prev_branch = branch_b
res = txn.fetchone()
branch_b = PduEntry(*res) if res else None
logger.debug("getting branch_b prev %s", pdu_tuple)
branch_b = self._get_pdu_tuple(txn, *pdu_tuple)
if branch_b:
branch_b = Pdu.from_pdu_tuple(branch_b)
logger.debug("branch_b=%s", branch_b)
......
tests/test_state.py
+ 140
− 45
View file @ 39e3fc69
......@@ -15,15 +15,18 @@
from twisted.internet import defer
from twisted.trial import unittest
from twisted.python.log import PythonLoggingObserver
from synapse.state import StateHandler
from synapse.storage.pdu import PduEntry
from synapse.federation.pdu_codec import encode_event_id
from synapse.federation.units import Pdu
from collections import namedtuple
from mock import Mock
import logging
import mock
......@@ -32,6 +35,11 @@ ReturnType = namedtuple(
)
def _gen_get_power_level(power_level_list):
def get_power_level(room_id, user_id):
return defer.succeed(power_level_list.get(user_id, None))
return get_power_level
class StateTestCase(unittest.TestCase):
def setUp(self):
self.persistence = Mock(spec=[
......@@ -40,6 +48,7 @@ class StateTestCase(unittest.TestCase):
"get_latest_pdus_in_context",
"get_current_state_pdu",
"get_pdu",
"get_power_level",
])
self.replication = Mock(spec=["get_pdu"])
......@@ -53,7 +62,9 @@ class StateTestCase(unittest.TestCase):
@defer.inlineCallbacks
def test_new_state_key(self):
# We've never seen anything for this state before
new_pdu = new_fake_pdu_entry("A", "test", "mem", "x", None, 10)
new_pdu = new_fake_pdu("A", "test", "mem", "x", None, "u")
self.persistence.get_power_level.side_effect = _gen_get_power_level({})
self.persistence.get_unresolved_state_tree.return_value = (
(ReturnType([new_pdu], []), None)
......@@ -76,8 +87,13 @@ class StateTestCase(unittest.TestCase):
# We do a direct overwriting of the old state, i.e., the new state
# points to the old state.
old_pdu = new_fake_pdu_entry("A", "test", "mem", "x", None, 10)
new_pdu = new_fake_pdu_entry("B", "test", "mem", "x", "A", 5)
old_pdu = new_fake_pdu("A", "test", "mem", "x", None, "u1")
new_pdu = new_fake_pdu("B", "test", "mem", "x", "A", "u2")
self.persistence.get_power_level.side_effect = _gen_get_power_level({
"u1": 10,
"u2": 5,
})
self.persistence.get_unresolved_state_tree.return_value = (
(ReturnType([new_pdu, old_pdu], [old_pdu]), None)
......@@ -95,14 +111,48 @@ class StateTestCase(unittest.TestCase):
self.assertFalse(self.replication.get_pdu.called)
@defer.inlineCallbacks
def test_overwrite(self):
old_pdu_1 = new_fake_pdu("A", "test", "mem", "x", None, "u1")
old_pdu_2 = new_fake_pdu("B", "test", "mem", "x", "A", "u2")
new_pdu = new_fake_pdu("C", "test", "mem", "x", "B", "u3")
self.persistence.get_power_level.side_effect = _gen_get_power_level({
"u1": 10,
"u2": 5,
"u3": 0,
})
self.persistence.get_unresolved_state_tree.return_value = (
(ReturnType([new_pdu, old_pdu_2, old_pdu_1], [old_pdu_1]), None)
)
is_new = yield self.state.handle_new_state(new_pdu)
self.assertTrue(is_new)
self.persistence.get_unresolved_state_tree.assert_called_once_with(
new_pdu
)
self.assertEqual(1, self.persistence.update_current_state.call_count)
self.assertFalse(self.replication.get_pdu.called)
@defer.inlineCallbacks
def test_power_level_fail(self):
# We try to update the state based on an outdated state, and have a
# too low power level.
old_pdu_1 = new_fake_pdu_entry("A", "test", "mem", "x", None, 10)
old_pdu_2 = new_fake_pdu_entry("B", "test", "mem", "x", None, 10)
new_pdu = new_fake_pdu_entry("C", "test", "mem", "x", "A", 5)
old_pdu_1 = new_fake_pdu("A", "test", "mem", "x", None, "u1")
old_pdu_2 = new_fake_pdu("B", "test", "mem", "x", None, "u2")
new_pdu = new_fake_pdu("C", "test", "mem", "x", "A", "u3")
self.persistence.get_power_level.side_effect = _gen_get_power_level({
"u1": 10,
"u2": 10,
"u3": 5,
})
self.persistence.get_unresolved_state_tree.return_value = (
(ReturnType([new_pdu, old_pdu_1], [old_pdu_2, old_pdu_1]), None)
......@@ -125,9 +175,15 @@ class StateTestCase(unittest.TestCase):
# We try to update the state based on an outdated state, but have
# sufficient power level to force the update.
old_pdu_1 = new_fake_pdu_entry("A", "test", "mem", "x", None, 10)
old_pdu_2 = new_fake_pdu_entry("B", "test", "mem", "x", None, 10)
new_pdu = new_fake_pdu_entry("C", "test", "mem", "x", "A", 15)
old_pdu_1 = new_fake_pdu("A", "test", "mem", "x", None, "u1")
old_pdu_2 = new_fake_pdu("B", "test", "mem", "x", None, "u2")
new_pdu = new_fake_pdu("C", "test", "mem", "x", "A", "u3")
self.persistence.get_power_level.side_effect = _gen_get_power_level({
"u1": 10,
"u2": 10,
"u3": 15,
})
self.persistence.get_unresolved_state_tree.return_value = (
(ReturnType([new_pdu, old_pdu_1], [old_pdu_2, old_pdu_1]), None)
......@@ -150,9 +206,15 @@ class StateTestCase(unittest.TestCase):
# We try to update the state based on an outdated state, the power
# levels are the same and so are the branch lengths
old_pdu_1 = new_fake_pdu_entry("A", "test", "mem", "x", None, 10)
old_pdu_2 = new_fake_pdu_entry("B", "test", "mem", "x", None, 10)
new_pdu = new_fake_pdu_entry("C", "test", "mem", "x", "A", 10)
old_pdu_1 = new_fake_pdu("A", "test", "mem", "x", None, "u1")
old_pdu_2 = new_fake_pdu("B", "test", "mem", "x", None, "u2")
new_pdu = new_fake_pdu("C", "test", "mem", "x", "A", "u3")
self.persistence.get_power_level.side_effect = _gen_get_power_level({
"u1": 10,
"u2": 10,
"u3": 10,
})
self.persistence.get_unresolved_state_tree.return_value = (
(ReturnType([new_pdu, old_pdu_1], [old_pdu_2, old_pdu_1]), None)
......@@ -175,10 +237,17 @@ class StateTestCase(unittest.TestCase):
# We try to update the state based on an outdated state, the power
# levels are the same but the branch length of the new one is longer.
old_pdu_1 = new_fake_pdu_entry("A", "test", "mem", "x", None, 10)
old_pdu_2 = new_fake_pdu_entry("B", "test", "mem", "x", None, 10)
old_pdu_3 = new_fake_pdu_entry("C", "test", "mem", "x", "A", 10)
new_pdu = new_fake_pdu_entry("D", "test", "mem", "x", "C", 10)
old_pdu_1 = new_fake_pdu("A", "test", "mem", "x", None, "u1")
old_pdu_2 = new_fake_pdu("B", "test", "mem", "x", None, "u2")
old_pdu_3 = new_fake_pdu("C", "test", "mem", "x", "A", "u3")
new_pdu = new_fake_pdu("D", "test", "mem", "x", "C", "u4")
self.persistence.get_power_level.side_effect = _gen_get_power_level({
"u1": 10,
"u2": 10,
"u3": 10,
"u4": 10,
})
self.persistence.get_unresolved_state_tree.return_value = (
(
......@@ -208,17 +277,23 @@ class StateTestCase(unittest.TestCase):
# triggering a get_pdu request
# The pdu we haven't seen
old_pdu_1 = new_fake_pdu_entry(
"A", "test", "mem", "x", None, 10, depth=0
old_pdu_1 = new_fake_pdu(
"A", "test", "mem", "x", None, "u1", depth=0
)
old_pdu_2 = new_fake_pdu_entry(
"B", "test", "mem", "x", "A", 10, depth=1
old_pdu_2 = new_fake_pdu(
"B", "test", "mem", "x", "A", "u2", depth=1
)
new_pdu = new_fake_pdu_entry(
"C", "test", "mem", "x", "A", 20, depth=2
new_pdu = new_fake_pdu(
"C", "test", "mem", "x", "A", "u3", depth=2
)
self.persistence.get_power_level.side_effect = _gen_get_power_level({
"u1": 10,
"u2": 10,
"u3": 20,
})
# The return_value of `get_unresolved_state_tree`, which changes after
# the call to get_pdu
tree_to_return = [(ReturnType([new_pdu], [old_pdu_2]), 0)]
......@@ -268,20 +343,27 @@ class StateTestCase(unittest.TestCase):
# triggering a get_pdu request
# The pdu we haven't seen
old_pdu_1 = new_fake_pdu_entry(
"A", "test", "mem", "x", None, 10, depth=0
old_pdu_1 = new_fake_pdu(
"A", "test", "mem", "x", None, "u1", depth=0
)
old_pdu_2 = new_fake_pdu_entry(
"B", "test", "mem", "x", "A", 10, depth=2
old_pdu_2 = new_fake_pdu(
"B", "test", "mem", "x", "A", "u2", depth=2
)
old_pdu_3 = new_fake_pdu_entry(
"C", "test", "mem", "x", "B", 10, depth=3
old_pdu_3 = new_fake_pdu(
"C", "test", "mem", "x", "B", "u3", depth=3
)
new_pdu = new_fake_pdu_entry(
"D", "test", "mem", "x", "A", 20, depth=4
new_pdu = new_fake_pdu(
"D", "test", "mem", "x", "A", "u4", depth=4
)
self.persistence.get_power_level.side_effect = _gen_get_power_level({
"u1": 10,
"u2": 10,
"u3": 10,
"u4": 20,
})
# The return_value of `get_unresolved_state_tree`, which changes after
# the call to get_pdu
tree_to_return = [
......@@ -357,20 +439,27 @@ class StateTestCase(unittest.TestCase):
# triggering a get_pdu request
# The pdu we haven't seen
old_pdu_1 = new_fake_pdu_entry(
"A", "test", "mem", "x", None, 10, depth=0
old_pdu_1 = new_fake_pdu(
"A", "test", "mem", "x", None, "u1", depth=0
)
old_pdu_2 = new_fake_pdu_entry(
"B", "test", "mem", "x", "A", 10, depth=2
old_pdu_2 = new_fake_pdu(
"B", "test", "mem", "x", "A", "u2", depth=2
)
old_pdu_3 = new_fake_pdu_entry(
"C", "test", "mem", "x", "B", 10, depth=3
old_pdu_3 = new_fake_pdu(
"C", "test", "mem", "x", "B", "u3", depth=3
)
new_pdu = new_fake_pdu_entry(
"D", "test", "mem", "x", "A", 20, depth=1
new_pdu = new_fake_pdu(
"D", "test", "mem", "x", "A", "u4", depth=1
)
self.persistence.get_power_level.side_effect = _gen_get_power_level({
"u1": 10,
"u2": 10,
"u3": 10,
"u4": 20,
})
# The return_value of `get_unresolved_state_tree`, which changes after
# the call to get_pdu
tree_to_return = [
......@@ -445,8 +534,13 @@ class StateTestCase(unittest.TestCase):
# We do a direct overwriting of the old state, i.e., the new state
# points to the old state.
old_pdu = new_fake_pdu_entry("A", "test", "mem", "x", None, 5)
new_pdu = new_fake_pdu_entry("B", "test", "mem", "x", None, 10)
old_pdu = new_fake_pdu("A", "test", "mem", "x", None, "u1")
new_pdu = new_fake_pdu("B", "test", "mem", "x", None, "u2")
self.persistence.get_power_level.side_effect = _gen_get_power_level({
"u1": 5,
"u2": 10,
})
self.persistence.get_unresolved_state_tree.return_value = (
(ReturnType([new_pdu], [old_pdu]), None)
......@@ -469,7 +563,7 @@ class StateTestCase(unittest.TestCase):
event = Mock()
event.event_id = "12123123@test"
state_pdu = new_fake_pdu_entry("C", "test", "mem", "x", "A", 20)
state_pdu = new_fake_pdu("C", "test", "mem", "x", "A", 20)
snapshot = Mock()
snapshot.prev_state_pdu = state_pdu
......@@ -496,13 +590,13 @@ class StateTestCase(unittest.TestCase):
)
def new_fake_pdu_entry(pdu_id, context, pdu_type, state_key, prev_state_id,
power_level, depth=0):
new_pdu = PduEntry(
def new_fake_pdu(pdu_id, context, pdu_type, state_key, prev_state_id,
user_id, depth=0):
new_pdu = Pdu(
pdu_id=pdu_id,
pdu_type=pdu_type,
state_key=state_key,
power_level=power_level,
user_id=user_id,
prev_state_id=prev_state_id,
origin="example.com",
context="context",
......@@ -514,6 +608,7 @@ def new_fake_pdu_entry(pdu_id, context, pdu_type, state_key, prev_state_id,
is_state=True,
prev_state_origin="example.com",
have_processed=True,
content={},
)
return new_pdu
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment