cmu-db / peloton

The Self-Driving Database Management System
http://pelotondb.io
Apache License 2.0
2.03k stars 623 forks source link

Failure when running YCSB with VALUE_PARAMETER supported #1394

Open pervazea opened 6 years ago

pervazea commented 6 years ago

This is trying to resolve #1382, which turns out to be a just a broken test.

Overview:

What works:

--- a/src/codegen/query_compiler.cpp +++ b/src/codegen/query_compiler.cpp @@ -124,7 +124,6 @@ bool QueryCompiler::IsExpressionSupported( const expression::AbstractExpression &expr) { switch (expr.GetExpressionType()) { case ExpressionType::STAR:

Peloton fails with:

2018-06-07 14:05:17 [/home/proj/pa_ss/peloton/src/executor/plan_executor.cpp:60:CompileAndExecutePlan] INFO  - query not in cache - compiling
2018-06-07 14:05:17 [/home/proj/pa_ss/peloton/src/executor/plan_executor.cpp:61:CompileAndExecutePlan] INFO  - UpdatePlan
2018-06-07 14:05:17 [/home/proj/pa_ss/peloton/src/codegen/lang/vectorized_loop.cpp:37:~VectorizedLoop] ERROR - You didn't call lang::VectorizedLoop::LoopEnd()!
2018-06-07 14:05:17 [/home/proj/pa_ss/peloton/src/codegen/function_builder.cpp:139:~FunctionBuilder] ERROR - Missing call to FunctionBuilder::ReturnAndFinish() for function '_18_pipeline_0_serialWork_seqscan_update_output'
2018-06-07 14:05:17 [/home/proj/pa_ss/peloton/src/codegen/function_builder.cpp:139:~FunctionBuilder] ERROR - Missing call to FunctionBuilder::ReturnAndFinish() for function '_18_plan'
2018-06-07 14:05:17 [/home/proj/pa_ss/peloton/src/executor/plan_executor.cpp:210:ExecutePlan] ERROR - Error thrown during execution: Attribute '' is not an available attribute
2018-06-07 14:05:17 [/home/proj/pa_ss/peloton/src/network/postgres_protocol_handler.cpp:824:ExecExecuteMessageGetResult] ERROR - Failed to execute: Attribute '' is not an available attribute

It continues, but eventually segfaults. I make no claim that the marshaling code is bug free. It has not been reviewed or systematically tested. IMO it should ideally be up in the network layer, but this was just an attempt to get codegen, sequential scans and prepared statements to co-exist.

pervazea commented 6 years ago

Code may be found in my prepstmt_seq_scan from https://github.com/pervazea/peloton.git

pervazea commented 6 years ago

To recap:

2018-06-11 10:18:24 [/home/proj/pa_ss/peloton/src/codegen/lang/vectorized_loop.cpp:37:~VectorizedLoop] ERROR - You didn't call lang::VectorizedLoop::LoopEnd()!

Prior to this.. the log looked like:

2018-06-11 10:18:24 [/home/proj/pa_ss/peloton/src/network/postgres_protocol_handler.cpp:372:ExecParseMessage] INFO  - PrepareStatement[S_6] => SELECT * FROM USERTABLE where YCSB_KEY=$1 FOR UPDATE^M
2018-06-11 10:18:24 [/home/proj/pa_ss/peloton/src/executor/plan_executor.cpp:73:CompileAndExecutePlan] INFO  - query found in cache^M
2018-06-11 10:18:24 [/home/proj/pa_ss/peloton/src/executor/plan_executor.cpp:77:CompileAndExecutePlan] INFO  - execute query^M
2018-06-11 10:18:24 [/home/proj/pa_ss/peloton/src/executor/plan_executor.cpp:85:CompileAndExecutePlan] INFO  - process query results^M

** this is probably the culprit
S_7 is the first "named" update statement  (5th one)

2018-06-11 10:18:24 [/home/proj/pa_ss/peloton/src/network/postgres_protocol_handler.cpp:325:ExecParseMessage] INFO  - S_7, UPDATE USERTABLE SET FIELD1=$1,FIELD2=$2,FIELD3=$3,FIELD4=$4,FIELD5=$5,FIELD6=$6,FIELD7=$7,FIELD8=$8,FIELD9=$9,FIELD10=$10 WHERE YCSB_KEY=$11
2018-06-11 10:18:24 [/home/proj/pa_ss/peloton/src/network/postgres_protocol_handler.cpp:372:ExecParseMessage] INFO  - PrepareStatement[S_7] => UPDATE USERTABLE SET FIELD1=$1,FIELD2=$2,FIELD3=$3,FIELD4=$4,FIELD5=$5,FIELD6=$6,FIELD7=$7,FIELD8=$8,FIELD9=$9,FIELD10=$10 WHERE YCSB_KEY=$11
pervazea commented 6 years ago

In UpdatePlan::Performbinding:

  auto *scan = static_cast<planner::AbstractScan *>(children[0].get());
  auto &col_ids = scan->GetColumnIds();
  for (oid_t col_id = 0; col_id < col_ids.size(); col_id++) {
    ais_.push_back(input_context.Find(col_id));
  }

We set ais_ with column attribute information. In the case in question, i.e. when JDBC switches to binary mode and tries to be smart about prepared statements, we get the follow behaviour:

  1. We go through and set attribute information in ais_
  2. When we switch to binary mode, we find that ais_ is already set. However, we go through this path again, and set more attributes.

In a test run with YCSB, the table has 11 columns. First time through:

  update_primary_key_ = false, 
  ais_ = std::vector of length 11, capacity 16 = {0x6180000cbc80, 
    0x6180000cbcb8, 0x6180000cbcf0, 0x6180000cbd28, 0x6180000cbd60, 
    0x6180000cbd98, 0x6180000cbdd0, 0x6180000cbe08, 0x6180000cbe40, 
    0x6180000cbe78, 0x6180000cbeb0}

Second time... table has ais_ already set, we set another 11 attributes, to produce:

  ais_ = std::vector of length 22, capacity 32 = {0x618000095c80, 
    0x618000095cb8, 0x618000095cf0, 0x618000095d28, 0x618000095d60, 
    0x618000095d98, 0x618000095dd0, 0x618000095e08, 0x618000095e40, 
    0x618000095e78, 0x618000095eb0, 0x61c00008d880, 0x61c00008d8b8, 
    0x61c00008d8f0, 0x61c00008d928, 0x61c00008d960, 0x61c00008d998, 
    0x61c00008d9d0, 0x61c00008da08, 0x61c00008da40, 0x61c00008da78, 
    0x61c00008dab0}

At this point things break.

In postgres_protocolhandler.cpp, approx. line 808, the function ExecExecuteMessage appears to do some statement caching for JDBC. (getting and setting the statement from portal).

Unsure of the right behavior, but a couple of thoughts:

  1. If we are keeping the plan around, with ais_ having been correctly set, we should not add to the attribute information.
  2. A much less desirable approach would be to clear ais_ and allow it to be re-initialized.
  3. This works with the executor, but not with codegen. Haven't examined why, possibly ais_ is not used by the executor.
pervazea commented 6 years ago

PLTestBase.java.txt UpdateTest.java.txt

The two files above allow this problem to be reproduced, using the JUnit tests

Temporarily:

pervazea commented 6 years ago

In today's Peloton meeting, Prashanth's assertion was that re-binding on the cached plan was not required.