feat!: optimize middleware querying #126

lukeromanowicz · 2023-04-17T15:55:05Z

Description

Resolves #12

Instead of querying all blockchain stats from REST API every time a new transaction message arrives from websocket, it calculates most of the statistics dynamically based on websocket data and syncs back with REST API only upon minting a new keyblock to correct possible misalignment caused by microforks. This implementation is utilizing message buffer because transactions and macroblock messages tend to arrive before the keyblock message. If there will be a hiccup in data transfer, the message buffer will reset itself after hitting 30 messages and resync everything through the REST API.

Breaking change: using middleware websocket v2 instead of v1. Change NUXT_PUBLIC_WEBSOCKET_URL value from wss://mainnet.aeternity.io/mdw/websocket to wss://mainnet.aeternity.io/mdw/v2/websocket.

Demo

The online demo might not work due to a change in the WebSocket environmental variable to use v2 websocket instead of v1

2023-04-18.10-52-19.mp4

Checklist:

I have read and followed the Contributing Guide
My change requires a change to the documentation.
I have updated the documentation accordingly. (.env.example file)

github-actions · 2023-04-17T15:58:33Z

Deployed to https://pr-126-aescan.stg.aepps.com

janmichek

Cool, seems like a lot of work 🥇 the change looks promising.
Could you also provide some measurements before and after change? How was the optimization successful?

src/stores/recentBlocks.js

janmichek · 2023-04-18T14:34:35Z

src/stores/recentBlocks.js

+  }
+
+  function processKeyblockUpdate(keyblock) {
+    if (keyblocks.value?.[0].hash !== keyblock.prev_key_hash) {


I would better read it in some human readable form of what's going on in here

Agree, I think this function should be more readable or at least have some comments. It's hard to understand what's going on.

Just like in other process* functions, updating stuff fully, partially, or skipping the update entirely depends on multiple variables that represent given scenario. I'm struggling to split it into smaller chunks without going into advanced programming patterns we haven't used in the project and producing a few times more code around it.

I added the comments where the code isn't very obvious. Please, let me know if it helps or I missed some spots.

Honestly, I am not satisfied with the outcome. The function is long and has many comments and it's hard to read for me

src/stores/recentBlocks.js

michele-franchi

Thanks for the hard work, this will optimise the app a lot.
Please have a look at my comments, some things are not clear to me and in general I understand the issues about handling websocket data so I would suggest to add some comments to some parts which could be hard to understand.

src/stores/recentBlocks.js

michele-franchi · 2023-04-19T08:55:55Z

src/stores/blockchainStats.js

@@ -44,5 +44,8 @@ export const useBlockchainStatsStore = defineStore('blockchainStats', {
      const { data } = await axios.get(`${useRuntimeConfig().public.MIDDLEWARE_URL}/v2/txs/count`)
      this.transactionsCount = data
    },
+    increaseTotalTransactionsCounter() {


I would rename this to increaseTransactionsCount but It's just a preference.

Thanks for your suggestion, adjusted.

michele-franchi · 2023-04-19T08:56:46Z

src/stores/recentBlocks.js

+      ...keyblock,
+    })
+
+    if (keyblocks.value.length > 18) {


Why are keblocks removed if the total length is > 18?

It's the limit of visibility, added a constant to make the value self-explanatory 👍

michele-franchi · 2023-04-19T08:57:38Z

src/stores/recentBlocks.js

+  }
+
+  function processKeyblockUpdate(keyblock) {
+    if (keyblocks.value?.[0].hash !== keyblock.prev_key_hash) {


Agree, I think this function should be more readable or at least have some comments. It's hard to understand what's going on.

michele-franchi · 2023-04-19T08:58:17Z

src/stores/recentBlocks.js

+        ...microblock,
+      })
+
+      if (selectedKeyblockMicroblocks.value.length > 30) {


Also here, I'm wondering why 30

It's the limit of visibility, added a constant to make the value self-explanatory 👍

lukeromanowicz · 2023-04-20T12:30:44Z

Demo showing the result:

Before (a few requests every time a new microblock is minted):

After (requests to REST API only when a new keyblock is minted):

janmichek

Thanks for the fixes, providing measurements and explanation. 🕵️ I wish there could be done something with the implementation. Some parts feel heavyweight and hard to read what's going on

src/stores/recentBlocks.js

janmichek · 2023-04-24T07:47:36Z

src/stores/recentBlocks.js

+  }
+
+  function processKeyblockUpdate(keyblock) {
+    if (keyblocks.value?.[0].hash !== keyblock.prev_key_hash) {


Honestly, I am not satisfied with the outcome. The function is long and has many comments and it's hard to read for me

src/stores/recentBlocks.js

src/pages/index.vue

lukeromanowicz · 2023-04-24T08:34:09Z

Thanks @janmichek I'll try to restructure the code right after I finish Oracle details page

…esponses

lukeromanowicz · 2023-04-26T15:19:52Z

@janmichek @michele-franchi I've improved reliability and readability. Please let me know what you think

janmichek

Very good edit ⚡ Hope you see the same value in code readability, even tho it was not easy to achieve given the fact of directing data from different sources. A long step forward I would say.
I would still like to see some nicely named constants of what's going on in some if expressions, but giving it the green light anyway 😺

src/stores/recentBlocks.js

lukeromanowicz · 2023-04-27T08:37:26Z

@janmichek I do, thanks for pushing for that. I had a brain lock when I tried to refactor it the first time and didn't know how to tackle it

michele-franchi · 2023-04-28T09:49:35Z

@csiarab could you check it?

CharisSiarampalis · 2023-04-28T13:53:29Z

@lukeromanowicz Is this normal ?

lukeromanowicz · 2023-05-01T10:25:40Z

@csiarab no, that's not normal. 👀 but also this part has not been touched in this PR and I'm not able to reproduce it. Please give it another try

michele-franchi

Really nice!

I noticed a difference between transaction count from the current dev:

Screen.Recording.2023-05-02.at.13.15.02.mov

and if you check the total transactions at the beginning of the video by a sum of all numbers inside microblocks you'll see it's not correct. Could you verify it? Thanks

…t on new keyblock mint

lukeromanowicz · 2023-05-03T08:59:54Z

@michele-franchi I'm afraid it's an issue that can be tackled only on the mdw side. There isn't much we can do about it right now. The current implementation we have on the develop branch is pretty much never accurate, while after changes in this PR it's accurate when a new keyblock gets minted. e.g.

I described the issues I found in a dedicated issue on the mdw side: aeternity/ae_mdw#1291
And created a follow-up ticket to improve our implementation once the backend side provides us with some tools to handle the issue in the frontend.

michele-franchi · 2023-05-04T07:48:57Z

@lukeromanowicz how is it accurate? In the screenshot I see Total transactions in the selected keyblock: 0 but there are two transactions. Is this still part of the issue?

lukeromanowicz · 2023-05-04T08:02:52Z

@michele-franchi There are two scenarios:

when you just open the page
both current solution and the new one are more or less equally inaccurate by approximately the amount of transactions that were added in the latest microblock
when you keep the page running until a new keyblock is minted
current solution is not accurate while websocket gets 100% accurate.

The screenshot I've sent above is proof that the current solution is not accurate. I'm discussing with the mdw team the possibility to fix this inaccuracy in the linked mdw issue

michele-franchi

Good job! I guess we can then merge this and followup incorrect transactions count.

feat: optimize middleware querying

293baa4

rewrite recentBlocks store to CAPI and add dynamic total tx counter

43a628f

lukeromanowicz requested review from michele-franchi and janmichek April 18, 2023 08:59

lukeromanowicz marked this pull request as ready for review April 18, 2023 09:34

janmichek requested changes Apr 18, 2023

View reviewed changes

michele-franchi requested changes Apr 19, 2023

View reviewed changes

lukeromanowicz added 2 commits April 20, 2023 13:07

Merge branch 'develop' into feat/optimize-data-querying

9bcffbc

improve websocket message handling naming

dce2e8d

lukeromanowicz requested review from janmichek and michele-franchi April 20, 2023 12:21

correct visibile keyblocks limit

0cb7a0b

janmichek requested changes Apr 24, 2023

View reviewed changes

lukeromanowicz added 4 commits April 26, 2023 09:22

Merge branch 'develop' into feat/optimize-data-querying

0c887c1

Merge branch 'develop' into feat/optimize-data-querying

3b80e36

optimize websocket communication readability

2473d84

reduce race condition between websocket initialization and REST API r…

3b81963

…esponses

lukeromanowicz requested a review from janmichek April 26, 2023 15:19

janmichek approved these changes Apr 27, 2023

View reviewed changes

src/stores/recentBlocks.js Show resolved Hide resolved

src/stores/recentBlocks.js Show resolved Hide resolved

michele-franchi reviewed May 2, 2023

View reviewed changes

lukeromanowicz added 2 commits May 2, 2023 14:25

Merge branch 'develop' into feat/optimize-data-querying

f82a958

reduce middleware race condition effect between REST API and websocke…

da71061

…t on new keyblock mint

lintfix

e83849b

lukeromanowicz requested a review from michele-franchi May 3, 2023 09:00

michele-franchi approved these changes May 8, 2023

View reviewed changes

lukeromanowicz merged commit 9fe8b35 into develop May 8, 2023

lukeromanowicz deleted the feat/optimize-data-querying branch May 8, 2023 05:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: optimize middleware querying #126

feat!: optimize middleware querying #126

lukeromanowicz commented Apr 17, 2023 •

edited

Loading

github-actions bot commented Apr 17, 2023

janmichek left a comment

janmichek Apr 18, 2023

michele-franchi Apr 19, 2023

lukeromanowicz Apr 20, 2023

janmichek Apr 24, 2023

michele-franchi left a comment •

edited

Loading

michele-franchi Apr 19, 2023

lukeromanowicz Apr 20, 2023

michele-franchi Apr 19, 2023

lukeromanowicz Apr 20, 2023

michele-franchi Apr 19, 2023

michele-franchi Apr 19, 2023

lukeromanowicz Apr 20, 2023

lukeromanowicz commented Apr 20, 2023 •

edited

Loading

janmichek left a comment

janmichek Apr 24, 2023

lukeromanowicz commented Apr 24, 2023

lukeromanowicz commented Apr 26, 2023

janmichek left a comment

lukeromanowicz commented Apr 27, 2023

michele-franchi commented Apr 28, 2023

CharisSiarampalis commented Apr 28, 2023

lukeromanowicz commented May 1, 2023

michele-franchi left a comment

lukeromanowicz commented May 3, 2023 •

edited

Loading

michele-franchi commented May 4, 2023

lukeromanowicz commented May 4, 2023 •

edited

Loading

michele-franchi left a comment

feat!: optimize middleware querying #126

feat!: optimize middleware querying #126

Conversation

lukeromanowicz commented Apr 17, 2023 • edited Loading

Description

Demo

Checklist:

github-actions bot commented Apr 17, 2023

janmichek left a 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

michele-franchi left a comment • edited Loading

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

lukeromanowicz commented Apr 20, 2023 • edited Loading

janmichek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukeromanowicz commented Apr 24, 2023

lukeromanowicz commented Apr 26, 2023

janmichek left a comment

Choose a reason for hiding this comment

lukeromanowicz commented Apr 27, 2023

michele-franchi commented Apr 28, 2023

CharisSiarampalis commented Apr 28, 2023

lukeromanowicz commented May 1, 2023

michele-franchi left a comment

Choose a reason for hiding this comment

lukeromanowicz commented May 3, 2023 • edited Loading

michele-franchi commented May 4, 2023

lukeromanowicz commented May 4, 2023 • edited Loading

michele-franchi left a comment

Choose a reason for hiding this comment

lukeromanowicz commented Apr 17, 2023 •

edited

Loading

michele-franchi left a comment •

edited

Loading

lukeromanowicz commented Apr 20, 2023 •

edited

Loading

lukeromanowicz commented May 3, 2023 •

edited

Loading

lukeromanowicz commented May 4, 2023 •

edited

Loading