added channel effective amount to the claim weight. #141

Merged
tiger5226 merged 2 commits from add_channel_weight into master 2019-02-25 03:52:38 +01:00
tiger5226 commented 2019-01-10 03:40:52 +01:00 (Migrated from github.com)
No description provided.
tzarebczan commented 2019-01-14 17:17:15 +01:00 (Migrated from github.com)

@tiger5226 mentioned we need to re-sync the data to get this populated, but he was hesitant because of previous issues with lighthouse + chainquery sync.

@tiger5226 mentioned we need to re-sync the data to get this populated, but he was hesitant because of previous issues with lighthouse + chainquery sync.
tiger5226 commented 2019-01-15 01:21:12 +01:00 (Migrated from github.com)

Yeah with so many claims now and the limits on node ( we should try to figure out how to increase this? ) I don't want it to fail and get in a restart loop. The solution here is to batch the sql query. Right now it is an http call which grabs all claims (when we do a full sync up). I would not expect that to be too many MB. We are already batching them up when inserting into elastic search. However, we never got to the root cause on dev.

Yeah with so many claims now and the limits on node ( we should try to figure out how to increase this? ) I don't want it to fail and get in a restart loop. The solution here is to batch the sql query. Right now it is an http call which grabs all claims (when we do a full sync up). I would not expect that to be too many MB. We are already batching them up when inserting into elastic search. However, we never got to the root cause on dev.
tiger5226 commented 2019-02-09 23:37:55 +01:00 (Migrated from github.com)

I misspoke above. This does require a full sync but it is best we just recreate the production environment altogether.

@nikooo777 We just need to rebuild with this branch so all the claims get synced problem and we can test. All claims must have this field otherwise it will error for normal searches. Once we verify we can do the DSN switch.

I misspoke above. This does require a full sync but it is best we just recreate the production environment altogether. @nikooo777 We just need to rebuild with this branch so all the claims get synced problem and we can test. All claims must have this field otherwise it will error for normal searches. Once we verify we can do the DSN switch.
nikooo777 (Migrated from github.com) approved these changes 2019-02-13 13:38:22 +01:00
nikooo777 (Migrated from github.com) left a comment

if it works then LGTM, but please check that last comment first

if it works then LGTM, but please check that last comment first
@ -72,0 +68,4 @@
'field_value_factor': {
'field' : 'effective_amount',
'factor' : effectiveFactor,
'missing': 1,
nikooo777 (Migrated from github.com) commented 2019-02-13 13:37:26 +01:00

are you sure this works? I'd expect it to be

          'source': `${effectiveFactor * doc['certificate_amount'].value}`,

(and same above)

are you sure this works? I'd expect it to be ```suggestion 'source': `${effectiveFactor * doc['certificate_amount'].value}`, ``` (and same above)
nikooo777 (Migrated from github.com) commented 2019-02-13 13:35:13 +01:00

I don't exactly know how it works in nodeJS but I bet interfaces would be suit well here to avoid this dry violation :p

I don't exactly know how it works in nodeJS but I bet interfaces would be suit well here to avoid this dry violation :p
nikooo777 (Migrated from github.com) commented 2019-02-13 13:35:50 +01:00

NBD though, it's probably just a waste of time right now

NBD though, it's probably just a waste of time right now
tiger5226 commented 2019-02-13 17:07:14 +01:00 (Migrated from github.com)

@nikooo777 It does not work at the moment. We need to rebuild ( see previous comment ) the elastic search database to test this change. So I committed your suggestion, I agree. We need to get an environment setup with these changes. Not sure it will work with that many claims so I have have to break the query up to get say 10K claims per query first.

@nikooo777 It does not work at the moment. We need to rebuild ( see previous comment ) the elastic search database to test this change. So I committed your suggestion, I agree. We need to get an environment setup with these changes. Not sure it will work with that many claims so I have have to break the query up to get say 10K claims per query first.
tiger5226 commented 2019-02-25 03:53:08 +01:00 (Migrated from github.com)

I found a solution where we can set a default value if it is missing.

I found a solution where we can set a default value if it is missing.
tiger5226 commented 2019-02-25 06:15:04 +01:00 (Migrated from github.com)

@nikooo777 ready for a rebuild now. Use the certificate_amount branch.

@nikooo777 ready for a rebuild now. Use the `certificate_amount` branch.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lighthouse.js#141
No description provided.