Leaderboard table #268

BernardHan · 2019-07-03T01:03:02Z

#267 #265

Create eventLeaderboard and globalLeaderboard tables and models
Make db helper methods for both tables
In sync, every bet, result set, vote amounts goes into eventLeaderboard investment
In sync, when event goes to WITHDRAWING status, update eventLeaderboard winnings and returnRatio
In sync, when event goes to WITHDRAWING status, update globalLeaderboard investments, winnings and returnRatio
Migration
Testings
Queries

src/models/event-leaderboard.js

src/models/global-leaderboard.js

src/db/db-helper.js

src/db/index.js

src/db/db-helper.js

src/sync/bet-placed.js

dwalintukan

please make the last changes then can merge. dont forget to rebase.

oh forgot the tests, lets wait on merging until tests are done.

src/models/global-leaderboard.js

src/models/event-leaderboard.js

src/db/index.js

src/sync/bet-placed.js

src/sync/event-status-withdrawing-changed.js

dwalintukan · 2019-07-04T04:29:35Z

src/sync/event-status-withdrawing-changed.js

+              // calculate winning for this user in this event
+              let winning = '0';
+              if (resultIndex === event.currentResultIndex) {
+                const res = await MultipleResultsEventApi.calculateWinnings({


we would still have the parallel limit issue here since the limit() is only for the outer promise. let me dig into this issue and lets leave this for now.

can we move the limit to wrap this part so no inner promises?
We query the bets and resultsets first, then start to limit.

Let's use https://caolan.github.io/async/v3/docs.html#eachOfSeries to handle the calculateWinnings and keep the outer limit promise. Would prefer not to limit the logic on the inside of an already parallel promise. Not too familiar with the logic of limit to start placing it in the middle of async parallel execution. Not too sure what would happen.

This will be slower since its running each calcWinnings individually but this ensures the concurrency limit is not broken.

src/sync/index.js

src/db/index.js

src/graphql/queries/leaderboard.js

src/graphql/schema.js

dwalintukan · 2019-07-08T03:35:42Z

src/sync/event-status-withdrawing-changed.js

+              // calculate winning for this user in this event
+              let winning = '0';
+              if (resultIndex === event.currentResultIndex) {
+                const res = await MultipleResultsEventApi.calculateWinnings({


Let's use https://caolan.github.io/async/v3/docs.html#eachOfSeries to handle the calculateWinnings and keep the outer limit promise. Would prefer not to limit the logic on the inside of an already parallel promise. Not too familiar with the logic of limit to start placing it in the middle of async parallel execution. Not too sure what would happen.

This will be slower since its running each calcWinnings individually but this ensures the concurrency limit is not broken.

BernardHan · 2019-07-09T01:52:35Z

src/sync/update-leaderboard.js

-              reject(err);
-            }
-          }));
+        async.forEachOf(filtered, (value, key, callback) => {


forgot to change to use eachOfSeries . Will change and test later.

please change before this gets merged, eachOf still is doing parallel exec.

dwalintukan · 2019-07-09T02:03:20Z

src/graphql/schema.js

@@ -418,19 +418,19 @@ type Query {
    skip: Int
  ): PaginatedBiggestWinner!

-  eventLeaderboard(
-    filter: LeaderboardFilter
+  eventLeaderboardEntries(


This should still be eventLeaderboard. Sorry wasn't clear. The eventLeaderboard query returns a list of LeaderboardEntrys. Like a users query will return a list of Users.

The relation is one-to-many. One eventLeaderboard query will return many LeaderboardEntrys.

Think eventLeaderboardEntries works better since we are querying for a set of paginated entries not the whole leaderboard.

In Event store, we only query for one specific entry just to get the winnings.

Fair enough. Actually it does conform to the other query names.

dwalintukan · 2019-07-09T02:03:27Z

src/graphql/schema.js


-  globalLeaderboard(
-    filter: LeaderboardFilter
+  globalLeaderboardEntries(


same thing here

dwalintukan · 2019-07-09T02:07:55Z

src/sync/update-leaderboard.js

-              reject(err);
-            }
-          }));
+        async.forEachOf(filtered, (value, key, callback) => {


please change before this gets merged, eachOf still is doing parallel exec.

dwalintukan · 2019-07-10T03:42:09Z

src/graphql/schema.js

@@ -418,19 +418,19 @@ type Query {
    skip: Int
  ): PaginatedBiggestWinner!

-  eventLeaderboard(
-    filter: LeaderboardFilter
+  eventLeaderboardEntries(


Fair enough. Actually it does conform to the other query names.

dwalintukan · 2019-07-10T03:47:10Z

src/graphql/queries/leaderboard.js

+
+  if (orderBy && orderBy.length > 0) {
+    const { field } = orderBy[0];
+    res.items.sort((a, b) => new BigNumber(b[field]).comparedTo(new BigNumber(a[field]))); // all descending


what if trying to sort by address?

nit: suggest to maybe use BN.js in this case. not sure if the performance impacts with BigNumber would come into play here.

field here is either investments or winnings, which are all converted from BigNumber instances, think it's better to keep it consistent to use BigNumber?

Need to handle what happens if someone passes in { field: 'eventAddress', direction: 'ASC' } or betterAddress. It will crash here since you're trying to convert an address to a BigNumber.

Was just worried about the performance of it. If you feel the performance is good compared to BN, then let's just keep it BigNumber. But would like to still use BN whenever possible to keep it standardized throughout the code.

ratio is float type, think it's better keep it consistent to use Bignumber still

I will skip the sorting if it's address

Ah right, ya if dealing w/ floats, then BigNumber all the way.

test/db/index.js

BernardHan requested review from dwalintukan and henryzbw July 3, 2019 01:03

BernardHan self-assigned this Jul 3, 2019

dwalintukan suggested changes Jul 3, 2019

View reviewed changes

dwalintukan approved these changes Jul 4, 2019

View reviewed changes

dwalintukan reviewed Jul 8, 2019

View reviewed changes

BernardHan changed the base branch from dev-062419 to dev-070419 July 9, 2019 01:50

BernardHan commented Jul 9, 2019

View reviewed changes

dwalintukan approved these changes Jul 9, 2019

View reviewed changes

dwalintukan approved these changes Jul 10, 2019

View reviewed changes

dwalintukan reviewed Jul 11, 2019

View reviewed changes

test/db/index.js Outdated Show resolved Hide resolved

test/db/index.js Show resolved Hide resolved

test/db/index.js Outdated Show resolved Hide resolved

test/db/index.js Outdated Show resolved Hide resolved

test/db/index.js Outdated Show resolved Hide resolved

BernardHan and others added 19 commits July 11, 2019 14:39

Add leaderboard tables and models

ced3256

Add helper methods for event leaderboard

c34899b

Add helper methods for global leaderboard

c644568

Index both tables

afb8496

Update event leaderboard investment when bet, result set, vote

5def03e

Make leaderboards updated when event status changes to withdrawing

eb29f51

Changes requested

24efc4e

Leaderboard update on withdrawing changed

579650d

Changes requested

95a3158

Add queries for leaderboard

714e2f3

Add tests

6a8cfb1

Changes requested

21298e5

Rewrite leaderboard update sync

9e61c68

Change to use each of series

934256a

Make leaderboard query sort properly

b3f5499

Add migration for eventleaderboard

c12230f

Migrate for global leaderboard

6aaa6d0

Move limit to outer loop

8b41d5a

Get rid of wrapper function

b625c60

Bowen Zhang and others added 2 commits July 11, 2019 14:40

Fix tests

a1ac0b4

Changes requested

deeea38

BernardHan force-pushed the leaderboard-table branch from 3696c89 to deeea38 Compare July 11, 2019 21:40

BernardHan merged commit 21c08ff into dev-070419 Jul 11, 2019

dwalintukan deleted the leaderboard-table branch July 12, 2019 03:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Leaderboard table #268

Leaderboard table #268

BernardHan commented Jul 3, 2019 •

edited by henryzbw

Loading

dwalintukan left a comment •

edited

Loading

dwalintukan Jul 4, 2019

BernardHan Jul 5, 2019

dwalintukan Jul 8, 2019 •

edited

Loading

dwalintukan Jul 8, 2019 •

edited

Loading

BernardHan Jul 9, 2019 •

edited

Loading

dwalintukan Jul 9, 2019

dwalintukan Jul 9, 2019

BernardHan Jul 9, 2019

dwalintukan Jul 10, 2019

dwalintukan Jul 9, 2019

dwalintukan Jul 9, 2019

dwalintukan Jul 10, 2019

dwalintukan Jul 10, 2019

dwalintukan Jul 10, 2019

BernardHan Jul 10, 2019

dwalintukan Jul 11, 2019

BernardHan Jul 11, 2019 •

edited

Loading

dwalintukan Jul 12, 2019

Leaderboard table #268

Leaderboard table #268

Conversation

BernardHan commented Jul 3, 2019 • edited by henryzbw Loading

dwalintukan 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

dwalintukan Jul 8, 2019 • edited Loading

Choose a reason for hiding this comment

dwalintukan Jul 8, 2019 • edited Loading

Choose a reason for hiding this comment

BernardHan Jul 9, 2019 • 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

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

BernardHan Jul 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BernardHan commented Jul 3, 2019 •

edited by henryzbw

Loading

dwalintukan left a comment •

edited

Loading

dwalintukan Jul 8, 2019 •

edited

Loading

dwalintukan Jul 8, 2019 •

edited

Loading

BernardHan Jul 9, 2019 •

edited

Loading

BernardHan Jul 11, 2019 •

edited

Loading