Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Review use of sequences (db, balance) #538

Closed
karmacoma opened this issue Apr 7, 2017 · 6 comments
Closed

Review use of sequences (db, balance) #538

karmacoma opened this issue Apr 7, 2017 · 6 comments
Assignees

Comments

@karmacoma
Copy link
Contributor

karmacoma commented Apr 7, 2017

Review use of sequences (db, balance) and drop use of them where not needed.

See: https://github.com/LiskHQ/lisk/blob/development/helpers/sequence.js

Belongs to: #449

@Tschakki
Copy link
Contributor

Tschakki commented Jan 10, 2018

As discussed, this issue is delayed for later, after 1.0.0 release.

@4miners
Copy link
Contributor

4miners commented Feb 18, 2018

Issue is still valid, sequences should be reviewed as follows:
We currently use 3 sequences (sequence, dbSequence, balancesSequence)

  • Check if we need both sequence and dbSequence
  • Check if usage is right - for example we shouldn't mix receive blocks with other API calls, as node can not receive blocks in time because of sequence is busy with other, not that important processing
  • Review if we need sequences for http API calls at all
  • Review if all processing of balances-related things (transactions) is performed through balancesSequence

@4miners 4miners reopened this Feb 18, 2018
@vitaly-t
Copy link
Contributor

vitaly-t commented Feb 18, 2018

In case this is useful, there is a promise-based sequence method within the spex library that's used by pg-promise internally. And that library's root can be accessed via pgp.spex, so no need to add any extra dependencies.

And if somebody decides to use, I will, of course help ;)

vitaly-t added a commit that referenced this issue Feb 19, 2018
...so that the rest of the library's API becomes accessible.

This is following [this comment](#538 (comment)), so the `spex` sub-module becomes easily accessible:

```js
const spex = require('./db').pgp.spex;

// now can use spex.sequence or any other methods.
```
@vitaly-t
Copy link
Contributor

Quickly threw in a PR to make that spex module accessible: #1587

@karmacoma
Copy link
Contributor Author

dbSequence can be removed. This is an odd artifact from early days of Lisk.

I think we should also rename other sequences to something like: blockSequence, poolSequence

Making sure use them sparingly and at the highest level in call stack, so that we do not run into a problem of co-dependent nested sequences.

@MaciejBaj
Copy link
Contributor

MaciejBaj commented Mar 16, 2018

dbSequence had been removed - #1715,
balancesSequence has been reviewed and the usages revised - #1730.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants