[INDY-1200] Each next 3PC batch have a timestamp not less than the previous one #646

lampkin-diet · 2018-04-24T13:41:07Z

Move tsRevocStore from indy-node
Set into this store now in "commit" action and in catchup procedure for all txn
Add "get_last_key" into storages
Each next 3PC batch have a timestamp not less than the previous one

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

…evious one Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

dsurnin · 2018-04-25T06:17:36Z

test this please

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

ashcherbakov · 2018-04-25T10:17:10Z

plenum/common/ledger_manager.py

+            req_handler = node.get_req_handler(ledgerId)
+            # Assume, that state updating was called before
+            # state
+            if req_handler and ledgerId == DOMAIN_LEDGER_ID:


Why do we do it here and not from postTxnFromCatchupAddedToLedger? Shouldn't it be called automatically when we update the state?
Looks like we also call it from ledger_req_handler's commit()...

ashcherbakov · 2018-04-25T10:18:18Z

plenum/server/ledger_req_handler.py

        self.state = state
        self.ledger = ledger
+        self.tsRevoc_store = tsRevoc_store


Why is it called tsRevoc_store? This looks like just a ts_store

ashcherbakov · 2018-04-25T10:21:48Z

plenum/server/replica.py

+
+    def _get_last_timestamp_from_state(self, ledger_id):
+        last_timestamp = None
+        if ledger_id == DOMAIN_LEDGER_ID:


I don't like too deep if-else hierarchy (more than 2).
I think the following pattern is more readable:

if not condition1: return do_something() if not condition2: return ....

Not so critical for this particular method since it's not so big.

ashcherbakov · 2018-04-25T10:23:22Z

plenum/test/node_catchup/test_tx_revoc_store_after_catchup.py

+    for reply in sdk_replies:
+        key = req_handler.prepare_buy_key(reply[1]['result']['identifier'],
+                                           reply[1]['result']['reqId'])
+        root_hash = req_handler.tsRevoc_store.get_equal_or_prev(reply[1]['result']['txnTime'])


Please add assert root_hash

ashcherbakov · 2018-04-25T10:24:33Z

plenum/test/replica/test_get_last_timestamp_from_state.py

+                                      sdk_wallet_steward,
+                                      1)[0][1]
+    reconnect_node_and_ensure_connected(looper, txnPoolNodeSet, node_to_disconnect)
+    ts_from_state = node_to_disconnect.master_replica._get_last_timestamp_from_state(DOMAIN_LEDGER_ID)


Should we wait for data equality here?

Yes, it would be added

ashcherbakov · 2018-04-25T10:28:27Z

plenum/test/replica/test_get_last_timestamp_from_state.py

+    req_handler = primary_node.getDomainReqHandler()
+    req_handler.tsRevoc_store.set(excpected_ts,
+                                  req_handler.state.headHash)
+    primary_node.master_replica.last_accepted_pre_prepare_time = None


Do we have a test where tm < last_accepted_pre_prepare_time, so we choose last_accepted_pre_prepare_time instead?

Yes, in plenum/test/replica/test_get_last_timestamp_from_state.py :: test_choose_ts_from_state

But we set last_accepted_pre_prepare_time to None here, so it's taken from the state.

ashcherbakov · 2018-04-25T10:30:33Z

storage/kv_store_rocksdb_int_keys.py

@@ -40,3 +40,12 @@ def get_equal_or_prev(self, key):
        except StopIteration:
            value = None
        return value
+
+    def get_last_key(self):


Please add a test to test_kv_storages

already added into storage/test/test_ts_store.py

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

…dger Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

…accepted Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

lampkin-diet · 2018-04-25T20:32:11Z

test this please

ashcherbakov · 2018-04-26T07:26:42Z

plenum/server/node.py

@@ -175,6 +176,8 @@ def __init__(self,

        self.primaryStorage = storage or self.getPrimaryStorage()

+        self.stateTsDbStorage = None


Please mention that this is stateTsStore for domain ledger

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

…evious one (hyperledger#646) * [INDY-1200] Move tsRevocStore from indy-node and support while catchup Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com> * [INDY-1200] Add get_last_key into storages Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com> * [INDY-1200] Each next 3PC batch have a timestamp not less than the previous one Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com> * [INDY-1200] flake8 fixes Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com> * [INDY-1200] add close for stateTsDbStorage Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com> * [INDY-1200] Syntax fix Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com> * [INDY-1200] Rename tsRevoc_store to ts_store Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com> * [INDY-1200] More readable changes Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com> * [INDY-1200] Refactor tests Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com> * [INDY-1200] Fix error Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com> * [INDY-1200] Move ts_store catchuping into postTxnFromCatchupAddedToLedger Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com> * [INDY-1200] tm now calculated from last accepted pptime if tm < last_accepted Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com> * [INDY-1200] Add comment for stateTsDbStorage Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

Andrew Nikitin added 4 commits April 24, 2018 10:32

[INDY-1200] Move tsRevocStore from indy-node and support while catchup

b2281cb

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

[INDY-1200] Add get_last_key into storages

5f74b6a

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

[INDY-1200] Each next 3PC batch have a timestamp not less than the pr…

4dae2f4

…evious one Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

[INDY-1200] flake8 fixes

0ec7ea2

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

Andrew Nikitin added 2 commits April 25, 2018 11:54

[INDY-1200] add close for stateTsDbStorage

32580b3

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

[INDY-1200] Syntax fix

a4a2038

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

ashcherbakov requested changes Apr 25, 2018

View reviewed changes

Andrew Nikitin added 6 commits April 25, 2018 16:57

[INDY-1200] Rename tsRevoc_store to ts_store

87d6ccf

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

[INDY-1200] More readable changes

516f109

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

[INDY-1200] Refactor tests

e163bbf

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

[INDY-1200] Fix error

3362676

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

[INDY-1200] Move ts_store catchuping into postTxnFromCatchupAddedToLe…

be88e7a

…dger Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

[INDY-1200] tm now calculated from last accepted pptime if tm < last_…

daf22bb

…accepted Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

ashcherbakov reviewed Apr 26, 2018

View reviewed changes

lampkin-diet changed the title ~~Public/indy 1200~~ [INDY-1200] Each next 3PC batch have a timestamp not less than the previous one Apr 26, 2018

Andrew Nikitin added 2 commits April 26, 2018 23:59

[INDY-1200] Add comment for stateTsDbStorage

456966a

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

Merge remote-tracking branch 'public/master' into public/indy-1200

0eaeb55

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

ashcherbakov approved these changes Apr 27, 2018

View reviewed changes

ashcherbakov merged commit 29c3c2a into hyperledger:master Apr 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[INDY-1200] Each next 3PC batch have a timestamp not less than the previous one #646

[INDY-1200] Each next 3PC batch have a timestamp not less than the previous one #646

lampkin-diet commented Apr 24, 2018

dsurnin commented Apr 25, 2018

ashcherbakov Apr 25, 2018

ashcherbakov Apr 25, 2018

ashcherbakov Apr 25, 2018

ashcherbakov Apr 25, 2018

ashcherbakov Apr 25, 2018

lampkin-diet Apr 25, 2018

ashcherbakov Apr 25, 2018

lampkin-diet Apr 25, 2018

ashcherbakov Apr 25, 2018

ashcherbakov Apr 25, 2018

lampkin-diet Apr 25, 2018

lampkin-diet commented Apr 25, 2018

ashcherbakov Apr 26, 2018

		@@ -175,6 +176,8 @@ def __init__(self,

		self.primaryStorage = storage or self.getPrimaryStorage()

		self.stateTsDbStorage = None

[INDY-1200] Each next 3PC batch have a timestamp not less than the previous one #646

[INDY-1200] Each next 3PC batch have a timestamp not less than the previous one #646

Conversation

lampkin-diet commented Apr 24, 2018

dsurnin commented Apr 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lampkin-diet commented Apr 25, 2018

Choose a reason for hiding this comment