Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move children_rshares2 out of consensus or get rid of it entirely #801

Closed
theoreticalbts opened this issue Jan 17, 2017 · 5 comments
Closed
Assignees
Milestone

Comments

@theoreticalbts
Copy link
Contributor

Basically children_rshares2 is not needed by consensus and I would imagine it adds quite a bit to reindex times as we have to do those tree traversals. Also it doesn't fit well architecturally with abstract comment rewards #774.

AFAICT the only thing it's used for is trending and trending30, which is only used for steemit.com. If we can build the selection of trending posts externally in JS / SQL without using the get_discussions_by_trending / get_discussions_by_trending30 method, we can remove children_rshares2 entirely.

If deleting it will make life too hard for the frontend team, let's at least try to remove the field from the core and put it in the tags plugin or something.

@roadscape
Copy link
Contributor

IMO, it's fine to sort trending by the top-level post's rshares only. However if we go this route [and even if we don't, in the wake of a separate comment reward pool] it would be extremely useful to have a separate call to fetch all trending comments. (also suggested here)

@theoreticalbts
Copy link
Contributor Author

On second thought I'm not sure this ticket is all that urgent. I originally thought this ticket would be a blocker for HF17 change #774, but that might not actually be the case.

@mvandeberg
Copy link
Contributor

This is definitely something we want to do. The goal is to eventually not need the Low Memory Node build option. However, I don't think it is something we need to work on for HF17.

@mvandeberg
Copy link
Contributor

We don't want to remove this until Steemit.com is no longer relying on steemd for this information.

@mvandeberg
Copy link
Contributor

children_rshares2 is not used by consensus and the ranking algorithms that were using it in the tags plugin have been updated. We should be able to safely remove this metic altogether.

@mvandeberg mvandeberg modified the milestones: 0.17.1, Future Improvements Mar 2, 2017
NateBrune added a commit that referenced this issue Apr 21, 2017
…nd undo regression from removing total_reward_shares2
NateBrune added a commit that referenced this issue Apr 25, 2017
mvandeberg pushed a commit that referenced this issue Apr 25, 2017
#801 remove children_rshares2 from consensus
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants