-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add vaxine wal replication endpoint #29
Conversation
5a45419
to
23c601b
Compare
@@ -56,6 +56,9 @@ | |||
get_txn_property/2 | |||
]). | |||
|
|||
-export_type([snapshot_time/0, dcid/0, clock_time/0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not in the header file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export_type
works in the very similar way as export for functions. There is no much value in exporting those types from all the modules that include corresponding hrl file. Rather if it's the same type much more helpful to reference it as antidote:dcid()
instead so that the reader knows that it's the same type used everywhere. Commonly it's the records that should go in the hrl files, for the rest of the types it's very questionable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, should eventually refactor the header file accordingly. I just would suggest to keep it in one place for the moment to avoid confusion. E.g. is antidote:dcid()
the same as dcid()
???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I guess the question that you probably ask here - wherever or not antidote:dcid()
is treated in the same way as dcid()
by tools like dialyzer. And honestly I do not know definite answer for that, so I try to use fully-qualified names to be on the safer side.
But also, defined type like this could have different definitions in different modules. So when I see the antidote:dcid()
type in multiple specs from different files I have a guaranty without checking other file definitions or hrl files, that it's the same type in both cases. While just dcid()
without module that exports it could have different definitions. So in order to know - I need to check definition. Maybe for dcid()
it's not that likely to end up in that situation, but some more general type like key()
it could very well be the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I meant for this very specific antidote:dcid()
: Is it the same thing as what is currently called dcid()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, yes, I didn't change dcid()
, just exported it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad - need more coffee first, before I read code...
maybe_notify([_], _) -> | ||
ok. | ||
|
||
while([H|T]) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this a normal fold? What is the difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, the fold is stopped, once an error occurred. I probably would have used a throw / catch instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's basically similar to do
while
. No accumulator is needed. In my view a bit cleaner then analog try catch implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that the throw/catch is the suggested way of doing it in Erlang. Probably, a matter of taste - for loops of this size it shouldn't matter much.
085cf3f
to
f9935eb
Compare
@v0idpwn fixed an issue with materialization we've been talking about, when multiple operations touching same key end up in the same trans |
Hmm we might have a race condition in the materialization part 🤔 |
…endpoint Commit adds write-ahead-log replication functionality together with vx_server and vx_client erlang applications. Vx_server serves as a top-level erlang application for vaxine project. Vx_client is intended to be used to communicating with vaxine from stand-alone applications. vx_server and vx_client communicate through erlang terms over tcp for now. SSL support and protobuf messaging to be added. Write-ahead-log replication functionality is intended for replicating materialized values from the antidote. This is the initial effort, and only single partition is supported for now. Another limitation is that replication always happens from the begging of the log, this to be changed in the future. Replication that is streamed from the server does not have any back-pressure at the moment, so vx_client is going to generate events as fast as it reads from the socket. On the Antidote site logging notification server concept was added to provide notification stream to continuosly trigger replication. This could be replaced by inotify-based notifications or other means in the future. disk_log:sync is only triggered for commit log records, and not for all the records as it used to be in the sync mode equal true. VAX-59, VAX-60, VAX-61, VAX-68, VAX-73
f9935eb
to
d5cee89
Compare
No description provided.