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

there may be a Bug with paging. #4

Open
thegenius opened this issue Apr 16, 2020 · 12 comments
Open

there may be a Bug with paging. #4

thegenius opened this issue Apr 16, 2020 · 12 comments
Assignees

Comments

@thegenius
Copy link

`let mut loop_index = 0;
let mut paging_state = None;
loop {
println!("loop index: {:?}", &loop_index);
println!("pagin state: {:?}", &paging_state);
let params = page_request.build_query_params(paging_state, None);
let body: ResponseBody = scylladb
.query_body(&page_request.row_query_str, params)
.await
.unwrap();

        let mut rows_cnt = 0;
        match &body {
            ResponseBody::Result(res) => {
                match res {
                    ResResultBody::Rows(rows) => {
                        rows_cnt = rows.rows_count;
                    },
                    _ => ()
                }
            },
            _ => (),
        }
        if rows_cnt == 0 {
            return Ok(PageResult {
                page_size: page_request.page_size,
                page_index: page_request.page_index,
                total: 0,
                data: Vec::new(),
            });
        }

        paging_state = body.as_rows_metadata().unwrap().paging_state;
        if page_request.page_index == loop_index {
            let rows = body.into_rows().unwrap();
            let mut result: Vec<K> = Vec::new();
            for row in rows {
                result.push(K::try_from_row(row).expect("row transform error"));
            }
            // dbg!(rows);
            // dbg!(paging_state);
            return Ok(PageResult {
                page_size: page_request.page_size,
                page_index: page_request.page_index,
                total: 0,
                data: result,
            });
        } else {
            loop_index = loop_index + 1;
        }
    }`
@thegenius
Copy link
Author

Query with QueryParams will block when page_size is other than 1. This can checked with very simple code

@AlexPikalov
Copy link
Owner

@thegenius thanks for the reporting. 👍
This is definitely something worth to check and fix.

@AlexPikalov AlexPikalov self-assigned this Apr 16, 2020
@AlexPikalov
Copy link
Owner

@thegenius

Perhaps you may want to check if the server has more page by using response metadata instead of

if rows_cnt == 0 {
  return
}

Please check pager implementation of the sync version https://github.com/AlexPikalov/cdrs/blob/master/src/cluster/pager.rs#L115.

With that logic in place you'd need to add an exit condition to the end of the loop step as in the sync example

@AlexPikalov
Copy link
Owner

Nevertheless pager helper is something that is definitely missed in async version.

@thegenius
Copy link
Author

thegenius commented Apr 18, 2020

I made a simple example, the query_body function will block when params is:
QueryParams { consistency: One, flags: [ PageSize, ], with_names: None, values: None, page_size: Some( 2, ), paging_state: None, serial_consistency: None, timestamp: None, }

query_body implement as below:

`async fn query_body(&mut self, query_str: &str, params: QueryParams) ->Result {

    println!("before query body");
    dbg!(&params);
    return match self.session.as_mut().query_with_params(&query_str, params).await {
        Ok(result) => match result.get_body() {
            Ok(body) => {
                println!("query success");
                dbg!(&body);
                return Ok(body)
            },
            Err(e) => {
                println!("query error");
                bail!(CqlDatabaseError::InvalidStmt {
                    msg: "error stmt".to_owned()
                })
            },
        },
        Err(e) => {
            println!("query error");
            bail!(CqlDatabaseError::InvalidStmt {
                msg: "error stmt".to_owned()
            })
        }
    };
}`

there are 4 rows in the table, and when page_size set to 1, the query_body function will not block;

@thegenius
Copy link
Author

OS: macOS
Database: ScyllaDB 3.2
Rust version: 1.41.0

@AlexPikalov
Copy link
Owner

👍 thanks! I'll check this out!

@Jasperav
Copy link

@krojew have you fixed this issue in your fork?

@krojew
Copy link

krojew commented Dec 15, 2020

@Jasperav to be honest - I don't know if this bug even exists in cdrs-tokio.

@Jasperav
Copy link

@krojew well I guess if the tests are green after merging AlexPikalov/cdrs#337 in your fork, there shouldn't be a paging bug (since I included a test in that PR that tests the paging functionality)

@krojew
Copy link

krojew commented Dec 15, 2020

@Jasperav I can confirm new tests pass.

@Jasperav
Copy link

@thegenius if you can switch over to krojew's fork and it also works for you, please close this issue

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

4 participants