Return Query Error for non existent DC

[samuelorji]

Return query error when there are no known nodes found during query planning

Fixes: #747

Pre-review checklist

• I have split my patch into logically separate commits.
• All commit messages clearly explain what they change and why.
• I added relevant tests for new features and bug fixes.
• All commits compile, pass static checks and pass test.
• PR description sums up the changes and reasons why they should be introduced.
• I have provided docstrings for the public items that I want to introduce.
• [-] I have adjusted the documentation in ./docs/source/.
• [-] I added appropriate Fixes: annotations to PR description.

[samuelorji]

The query_plan is an iterator that provides a Node, but this iterator can be empty due to the inability to find/pick a node which is mostly caused here.

There could be other reasons why the iterator could be empty, but I can't see any yet

[samuelorji]

@Lorak-mmk. I can see that the Cassandra tests are failing on CI, but they work locally for me. Thoughts?

[samuelorji]

Wanted to do a peekable check to ensure the iterator is not empty like:

query_plan.peekable()

but it caused this lifetime error:

error: higher-ranked lifetime error
   --> scylla/src/transport/session_test.rs:409:5
    |
409 | /     tokio::spawn(async move {
410 | |         let values = (
411 | |             (1_i32, 2_i32, "abc"),
412 | |             (),
...   |
415 | |         session_clone.batch(&batch, values).await.unwrap();
416 | |     })
    | |______^
    |
    = note: could not prove `[async block@scylla/src/transport/session_test.rs:409:18: 416:6]: std::marker::Send`
    ```
    So, I just defaulted to doing this more simple check

[Lorak-mmk]

The tests fail because they run Scylla on different IP. Please take a look at other tests in this file - they use create_new_session_builder() to get a session builder with correct IP. You can then call

        .default_execution_profile_handle(handle)
        .build()
        .await
        .expect("cannot create session");

on resulting builder.

[samuelorji]

Thanks

[Lorak-mmk]

Also, please use unique_keyspace_name(); to prevent future conflicts with other tests. I know that the query is supposed to fail, but if we introduce a regression and it doesn't fail, it may cause other tests to fail, making it harder to understand and fix.

[piodul]

Notice that execute_query returns None in case there were no nodes tried. The None is converted into QueryError::ProtocolError("Empty query plan - driver bug!") at some point - look those places up by grepping on the error message. I don't like the modifications made to execute_query because now

• The function never returns None, but it has Option in its return type,
• The new behavior will break speculative execution (look at speculative_execution.rs).

I don't think that execute_query needs to be touched at all, it's just that the error message needs to be changed. You should go over the places where Empty query plan - driver bug! is returned, change it to the new error variant you introduced and that should be it.

[wprzytula]

@samuelorji what's the status of this?