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

Semantics of apply_updates is unclear #372

Open
mihaibudiu opened this issue Sep 20, 2019 · 8 comments
Open

Semantics of apply_updates is unclear #372

mihaibudiu opened this issue Sep 20, 2019 · 8 comments
Assignees
Labels
bug Something isn't working good first issue (Rust) Good first issue that requires the knowledge of Rust, but not Haskell.

Comments

@mihaibudiu
Copy link

Although a transaction involving apply_updates can fail with an exception, some of the updates in the transaction may still be applied! What updates are applied? Shouldn't the transaction be in fact aborted?

@ryzhyk
Copy link
Contributor

ryzhyk commented Sep 20, 2019

This should not happen. Do you have a script to reproduce this?

@mihaibudiu
Copy link
Author

You cannot test this in a script, because it stops when a transaction fails.
I have a java program.

@mihaibudiu
Copy link
Author

import java.util.*;
import ddlogapi.DDlogAPI;
import ddlogapi.DDlogCommand;
import ddlog.x.*;
import ddlogapi.DDlogException;

public class XTest {
    private final DDlogAPI api;

    XTest() throws DDlogException {
        this.api = new DDlogAPI(1, null, false);
    }

    void onCommit(DDlogCommand command) {
        System.out.println(command.toString());
    }

    @FunctionalInterface
    public interface CheckedRunnable {
        void run() throws DDlogException;
    }

    void expectFail(String expected, CheckedRunnable r) {
        String message = null;
        try {
            r.run();
        } catch (Exception ex) {
            message = ex.getMessage();
            System.err.println("Message=" + message);
        }
        if (message == null)
            throw new RuntimeException("No message receieved with exception");
        if (!message.contains(expected))
            throw new RuntimeException("Error does not look like `" + expected + "'\n" +
                                       "error is `" + message + "'");
    }

    void run() throws DDlogException {
        {
            // Insert the same key twice
            xUpdateBuilder builder = new xUpdateBuilder();
            builder.insert_R0(10, 12);
            builder.insert_R0(10, 13);
            this.api.transactionStart();
            this.expectFail("duplicate key", () -> builder.applyUpdates(this.api) );
            this.api.transactionCommitDumpChanges(this::onCommit);
        }
        {
            xUpdateBuilder builder = new xUpdateBuilder();
            builder.insert_R0(10, 12);
            this.api.transactionStart();
            builder.applyUpdates(this.api);
            this.api.transactionCommitDumpChanges(this::onCommit);
        }

        this.api.stop();
    }

    public static void main(String[] args) throws DDlogException {
        XTest test = new XTest();
        test.run();
    }
}

And the corresponding DDlog:

input relation R0(key: bit<16>, value: bit<16>) primary key (k) k.key
output relation O0(key: bit<16>, value: bit<16>)
O0(k, v) :- R0(k, v).

You may also want the script to run this.

@mihaibudiu
Copy link
Author

The second transaction fails because the first one has already inserted the first tuple - although the first transaction fails.

@mihaibudiu
Copy link
Author

I can submit this as a standalone test and write separate tests for other cases.
Right now it's part of a larger test.

@ryzhyk
Copy link
Contributor

ryzhyk commented Sep 20, 2019

Ok, now I see what you mean. The first transaction was successfully committed with only the first update being applied. If you wanted the first transaction to fail cleanly, you should have called transactionRollback. But I agree this is not good: one would expect applyUpdates to succeed or fail atomically.

@ryzhyk ryzhyk self-assigned this Sep 20, 2019
@mihaibudiu
Copy link
Author

I will submit this test separately

@mihaibudiu mihaibudiu added the bug Something isn't working label Sep 20, 2019
@ryzhyk
Copy link
Contributor

ryzhyk commented Feb 17, 2020

The current behavior was documented in fcec8c2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue (Rust) Good first issue that requires the knowledge of Rust, but not Haskell.
Projects
None yet
Development

No branches or pull requests

2 participants