Skip to content

refactor: replace Redis by custom implementation#8

Merged
hacdias merged 13 commits intomasterfrom
no-redis
Sep 24, 2021
Merged

refactor: replace Redis by custom implementation#8
hacdias merged 13 commits intomasterfrom
no-redis

Conversation

@hacdias
Copy link
Copy Markdown
Member

@hacdias hacdias commented May 6, 2021

Closes #7.

@hacdias hacdias changed the title replace redis refactor: replace Redis by custom implementation May 8, 2021
@hacdias
Copy link
Copy Markdown
Member Author

hacdias commented May 8, 2021

@nonsense here's a suggested approach for removing Redis and replacing it by a custom implementation. I left 2 comments on the places that I'm most unsure about. I didn't test it with testground yet, but as tests are passing here it should work. We also need to check the performance.

Do you think you could leave a review up until Wednesday?

@hacdias hacdias requested a review from nonsense May 15, 2021 06:27
@hacdias hacdias marked this pull request as ready for review May 15, 2021 07:04
@hacdias
Copy link
Copy Markdown
Member Author

hacdias commented May 15, 2021

Looking at the PR on the main repository, it seems that all tests work out well with this version. We still need to do the stress tests to make sure there's no performance regressions.

@nonsense
Copy link
Copy Markdown
Member

@hacdias this LGTM. I added a few comments. I also think we should test with Testground as well, and not rely solely on the tests here. This is a new service, and not yet battle-tested, so that we rely so much on the unit tests.

Could you test with:

  • local:docker
  • local:exec
  • cluster:k8s (lotus-soup and storm and network/ping-pong testplans should be enough - I can help with that)

hacdias and others added 9 commits May 27, 2021 16:04
Co-authored-by: Anton Evangelatov <anton.evangelatov@gmail.com>
Co-authored-by: Anton Evangelatov <anton.evangelatov@gmail.com>
Co-authored-by: Anton Evangelatov <anton.evangelatov@gmail.com>
Co-authored-by: Anton Evangelatov <anton.evangelatov@gmail.com>
License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
@hacdias
Copy link
Copy Markdown
Member Author

hacdias commented May 29, 2021

I will test the service with the different runners next week. I may need some help with k8s. For the record, I tested with local:docker before opening this PR and it worked. But I will make sure to test again with both locals.

@hacdias
Copy link
Copy Markdown
Member Author

hacdias commented Jun 3, 2021

In addition to testing, I want to mention that testground/testground#1187 is using this PR and testing those runners. However, I am now going to test the local runners.

@hacdias
Copy link
Copy Markdown
Member Author

hacdias commented Jun 3, 2021

Tested with:

  • local:docker
    • network ping-pong 2 instances
    • splitbrain accept 20 / 50 instances
  • local:exec
    • example sync 20 / 50 / 100 instances

(can be updated)

@hacdias
Copy link
Copy Markdown
Member Author

hacdias commented Jul 1, 2021

@nonsense executed network/ping-pong and example/sync (100 instances) on k8s without issues. Lotus _compositions/baseline-k8s-1-1.toml also run well.

@hacdias hacdias requested a review from nonsense July 2, 2021 09:25
@hacdias hacdias merged commit eacab3b into master Sep 24, 2021
@hacdias hacdias deleted the no-redis branch September 24, 2021 11:42
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

Successfully merging this pull request may close these issues.

Replace Redis by custom implementation

2 participants