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

Test for ordering of log #19

Closed
bieniusa opened this issue May 23, 2014 · 8 comments
Closed

Test for ordering of log #19

bieniusa opened this issue May 23, 2014 · 8 comments

Comments

@bieniusa
Copy link
Contributor

Responsible: Manuel
Collaborator: Alejandro

@angbrav
Copy link
Contributor

angbrav commented May 30, 2014

I have checked this property in dets.

It works as expected if we use insert_next instead of insert. The problem of using insert is that in case of trying to insert duplicates, it will move the duplicated operation to the tail of the list instead of leaving it where it was originally placed.

For instance, lets say we insert operations in the following order: op1 -> op3 -> op4 -> op2 for key floppy. A look up over floppy key will return [op1,op3,op4,op2]. But if we insert op3 again, by using the regular insert, the lookup would return [op1, op4, op2, op3] while using insert_new, the look up will still return [op1,op3,op4,op2] which the desired output.

I have changed all inserts by insert_new. Please keep this on mind.

I will close this issue

@angbrav angbrav closed this as completed May 30, 2014
@angbrav
Copy link
Contributor

angbrav commented Jun 1, 2014

I was only half right!

I was right saying that by using insert, the order is not preserved when duplicate inserts. Nevertheless, I was wrong saying that insert_new solves the problem.

I have modify it once more. Now, before inserting, the logging_vnode first check whether the operation has been already log. In case it is already logged, it just ignores the operations and return success.

I am pretty sure it works properly now.

@angbrav angbrav reopened this Jun 1, 2014
@cmeiklejohn
Copy link
Contributor

It would be super nice to write a QuickCheck property for this.

@cmeiklejohn
Copy link
Contributor

Can you just write a quick riak_test to assert this behavior is true so we can prevent against regressions? Then we will close this one out.

@angbrav
Copy link
Contributor

angbrav commented Jun 23, 2014

Actually, I think I cannot do it. The tricky part of this is when inserting duplicated operations. From the API there is no way to do that so I cannot generate a test. Plus, the API only allows us to get the object materialised, in order to further test this from riak_test, I would need to expose an operation that returns the list of operations without materialize.

Please correct me if I am wrong and give some hints on how to do it.

@angbrav
Copy link
Contributor

angbrav commented Jun 24, 2014

I was rethinking ad retesting this. The current implementation only ensures order per key and not per partition. Do we need it per partition? In that case I think we would need to modify the way we store thinks because in the way we do it now and using sets, I would say is not possible.

@bieniusa
Copy link
Contributor Author

bieniusa commented Jul 8, 2014

Probably fixed with the new log version.

@bieniusa bieniusa closed this as completed Jul 8, 2014
@cmeiklejohn
Copy link
Contributor

Marking this finished, as it related to the older version of the logging virtual node.

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

3 participants