V6/paranet sync command #3145
V6/paranet sync command #3145
Conversation
aleksamagicka
force-pushed
the
v6/paranet-sync-command-aleksa
branch
from
358e0a7 to
0946b5d
Compare
aleksamagicka
requested review from
branarakic,
djordjekovac,
NZT48 and
u-hubar
as code owners
aleksamagicka
force-pushed
the
v6/paranet-sync-command-aleksa
branch
from
9004f0c to
437c87f
Compare
src/modules/repository/implementation/sequelize/models/paranet.js
Outdated
Show resolved
Hide resolved
| }; | ||
|
|
||
| promises.append( | ||
| this.commandExecutor.add({ |
There was a problem hiding this comment.
I think that it would be better to change the logic here. This command will potentially schedule a lot of paranet-sync-commands (if we have a lot of assets in Paranet and node started syncing it for the first time.). Previously we had a problem when scheduling a lot of commands in command executor - operational db become too slow...
I recommend to change the logic so that start-paranet-sync-command is scheduled as permanent command and executed just once during node start. It's purpose is to schedule new paranet-sync-command for each Paranet user defined in configuration.
paranet-sync-command should have logic to get and store all assets connected to that paranet.
We did logic like this in epoch-check-command/blockchain-epoch-check-command and get-latest-service-agreement-data/blockchain-get-latest-service-agreement-data please check it out.
| await this.repositoryModuleManager.updateParanetKaCount(paranetId, contractKaCount); | ||
| }); | ||
|
|
||
| await Promise.all(promises); |
There was a problem hiding this comment.
What will happen here if one of the promises fails, we will update Paranet ka count in database but not schedule all commands, so we will miss some token ids
| async syncAsset(blockchain, contract, tokenId, assertionIds, stateIndex, paranetId, paranetRepository, latestAsset, deleteFromEarlier) { | ||
| try { | ||
| if ( | ||
| await this.repositoryModuleManager.isStateSynced( |
There was a problem hiding this comment.
You are missing implementation for couple of methods in repository module used in this command
src/modules/repository/implementation/sequelize/repositories/paranet-repository.js
Outdated
Show resolved
Hide resolved
| ); | ||
|
|
||
| // Go through all except the last one | ||
| for (let stateIndex = assertionIds.length - 2; stateIndex > 0; stateIndex -= 1) { |
There was a problem hiding this comment.
Should loop condition be >0, if we have assertionIds.length equal to 2, would we want to get asset stateIndex 0?
There was a problem hiding this comment.
Or state index starts from 1?
There was a problem hiding this comment.
Initial code was this, so I'm not sure
There was a problem hiding this comment.
Initial state index of the asset is 0, if there is one update it's assertionIds.length will be 2. When we call this command it won't get into the lope, as stateIndex will be 0 at beginning of the loop, and it should process this old asset and put it historical, but with current condition we won't do it?
There was a problem hiding this comment.
We should test this :D
| @@ -96,6 +101,60 @@ class GetService extends OperationService { | |||
| tokenId, | |||
| keyword, | |||
| ); | |||
|
|
|||
| if (paranetSync) { | |||
There was a problem hiding this comment.
Generally not sure if this is the right place for sync logic.
I think it shouldn't be the responsibility of getting service to do asset sync logic, I would suggest moving this logic to paranet-sync-command and just getting the result of the network get from there, and continue inserting and moving data.
There was a problem hiding this comment.
IIRC Djordje said it's OK, as it's already syncing assets above?
aleksamagicka
force-pushed
the
v6/paranet-sync-command-aleksa
branch
from
18143ea to
3c90e3b
Compare
|
Where are we creating new paranets repositories in triple store? |
In ot-node.js |
Mihajlo-Pavlovic
merged commit fe88963
into
OriginTrail:v6/paranets
No description provided.