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

Rearchitect pulse mechanism #356

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Conversation

lolski
Copy link
Member

@lolski lolski commented Sep 21, 2021

What is the goal of this PR?

We've moved the pulse mechanism from session to client. Architecturally it makes more sense - the pulse is a mechanism to detect liveness and it's the client that may crash or gets cut off from the network, not any particular individual session instance owned by the client.

What are the changes implemented in this PR?

  • Move the pulse mechanism from session to client
  • Replace various superfluous create methods with regular constructors

@lolski lolski self-assigned this Sep 22, 2021
@lolski lolski added this to the 2.5.0 milestone Sep 22, 2021
@lolski lolski changed the title Client pulse Replace session pulse with client pulse Sep 22, 2021
@@ -59,13 +59,27 @@
import static org.junit.Assert.assertEquals;

@SuppressWarnings("Duplicates")

class ClientQueryTest2 {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: remove

@@ -45,8 +45,8 @@ def vaticle_dependencies():
def vaticle_typedb_protocol():
git_repository(
name = "vaticle_typedb_protocol",
remote = "https://github.com/vaticle/typedb-protocol",
commit = "3300d136a35f35bed58137bb79a6e3774dd170b9", # sync-marker: do not remove this comment, this is used for sync-dependencies by @vaticle_typedb_protocol
remote = "https://github.com/lolski/typedb-protocol",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: update

connection/core/CoreClient.java Outdated Show resolved Hide resolved
connection/TypeDBClientImpl.java Outdated Show resolved Hide resolved
boolean alive;
try {
alive = stub().clientPulse(pulseReq(ID())).getAlive();
System.out.println("client pulse - alive: " + alive);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: remove log statement

@lolski lolski changed the title Replace session pulse with client pulse Rearchitect pulse mechanism Oct 1, 2021
@lolski lolski removed this from the 2.5.0 milestone Oct 13, 2021
@lolski lolski added this to the Technical Debt milestone Nov 1, 2021
@flyingsilverfin flyingsilverfin force-pushed the master branch 2 times, most recently from 9de6052 to 5932aad Compare March 23, 2024 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant