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

[YCQL] ALTER TABLE DROP/RENAME column should not impact index using that column #1386

Closed
ndeodhar opened this issue May 15, 2019 · 5 comments

Comments

@ndeodhar
Copy link
Contributor

commented May 15, 2019

No description provided.

@ndeodhar ndeodhar added the kind/bug label May 15, 2019

@ndeodhar ndeodhar added this to To Do in Query Language via automation May 15, 2019

@ndeodhar

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

Based on @frozenspider's testing:

cqlsh:k> CREATE TABLE ti1(pk int primary key, i1 int, i2 int, i3 int) WITH transactions = { 'enabled' : true };
cqlsh:k> CREATE UNIQUE INDEX ti1_uidx ON ti1(i1) INCLUDE (i2);
cqlsh:k> ALTER TABLE ti1 DROP  i2;
Warning: schema version mismatch detected; check the schema versions of your nodes in system.local and system.peers.
Schema metadata was not refreshed. See log for details.

Master dies with:

F20190515 14:17:55 ../../src/yb/master/yql_indexes_vtable.cc:30] Check failed: column.ok() 
    @        0x11000b380  google::LogDestination::LogToSinks()
    @        0x11000a3de  google::LogMessage::SendToLog()
    @        0x11000ad55  google::LogMessage::Flush()
    @        0x11000f80f  google::LogMessageFatal::~LogMessageFatal()
    @        0x11000bd29  google::LogMessageFatal::~LogMessageFatal()
    @        0x10435cf6c  yb::master::(anonymous namespace)::ColumnName()
    @        0x10435bcfe  yb::master::YQLIndexesVTable::RetrieveData()
    @        0x1042ee3e2  yb::master::YQLVirtualTable::GetIterator()
    @        0x106d5f92f  yb::docdb::QLReadOperation::Execute()
    @        0x105faa00a  yb::tablet::AbstractTablet::HandleQLReadRequest()
    @        0x1042cae6c  yb::master::SystemTablet::HandleQLReadRequest()
    @        0x10576d825  yb::tserver::TabletServiceImpl::DoRead()
    @        0x10576bd23  yb::tserver::TabletServiceImpl::CompleteRead()
    @        0x10576a675  yb::tserver::TabletServiceImpl::Read()
    @        0x10bb9da16  yb::tserver::TabletServerServiceIf::Handle()
    @        0x10d0cb5c2  yb::rpc::ServicePoolImpl::Handle()
    @        0x10d0cddb1  yb::rpc::ServicePoolImpl::Handle()
    @        0x10cfa37a7  yb::rpc::InboundCall::InboundCallTask::Run()
    @        0x10d0f637d  yb::rpc::(anonymous namespace)::Worker::Execute()
    @        0x10d0f7e91  _ZNSt3__1L8__invokeIRMN2yb3rpc12_GLOBAL__N_16WorkerEFvvERPS4_JEvEEDTcldsdeclsr3std3__1E7forwardIT0_Efp0_Efp_spclsr3std3__1E7forwardIT1_Efp1_EEEOT_OSA_DpOSB_
    @        0x10d0f7e10  _ZNSt3__1L15__apply_functorIMN2yb3rpc12_GLOBAL__N_16WorkerEFvvENS_5tupleIJPS4_EEEJLm0EENS7_IJEEEEENS_13__bind_returnIT_T0_T2_Xsr22__is_valid_bind_returnISC_SD_SE_EE5valueEE4typeERSC_RSD_NS_15__tuple_indicesIJXspT1_EEEEOSE_
    @        0x10d0f7dbc  _ZNSt3__16__bindIRKMN2yb3rpc12_GLOBAL__N_16WorkerEFvvEJRKPS4_EEclIJEEENS_13__bind_returnIS6_NS_5tupleIJS9_EEENSF_IJDpOT_EEEXsr22__is_valid_bind_returnIS6_SG_SK_EE5valueEE4typeESJ_
    @        0x10d0f7d5d  _ZNSt3__1L8__invokeIRNS_6__bindIRKMN2yb3rpc12_GLOBAL__N_16WorkerEFvvEJRKPS5_EEEJEEEDTclclsr3std3__1E7forwardIT_Efp_Espclsr3std3__1E7forwardIT0_Efp0_EEEOSF_DpOSG_
    @        0x10d0f7d0d  _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRNS_6__bindIRKMN2yb3rpc12_GLOBAL__N_16WorkerEFvvEJRKPS7_EEEEEEvDpOT_
    @        0x10d0f6af1  _ZNSt3__110__function6__funcINS_6__bindIRKMN2yb3rpc12_GLOBAL__N_16WorkerEFvvEJRKPS6_EEENS_9allocatorISE_EEFvvEEclEv
    @        0x1057c9585  std::__1::function<>::operator()()
    @        0x10ed7188a  yb::Thread::SuperviseThread()
    @     0x7fff5ee152eb  _pthread_body
    @     0x7fff5ee18249  _pthread_start
    @     0x7fff5ee1440d  thread_start
@nocaway

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2019

After ALTER TABLE execution, the database is corrupted and servers are no longer available. This bug should be fixed asap to avoid data corruption.

@ndeodhar

This comment has been minimized.

Copy link
Contributor Author

commented Jul 30, 2019

Lets handle this in two phases:

  1. PHASE 1: Disallow dropping a column that's used in an index. To do this, the index needs to be first dropped and then the column should be dropped.
  2. PHASE 2: Support CASCADE mode which will drop both the index and the column.

@d-uspenskiy d-uspenskiy moved this from To Do to In progress in Query Language Jul 31, 2019

d-uspenskiy added a commit that referenced this issue Aug 6, 2019
ENG-5285 #1386 [YCQL] Do not allow to drop indexed column
Summary:
Dropping of column covered by index can cause data corruption.
To prevent that problem check is added that column is not covered by index in case of
dropping.
Check must be removed when problem will be fixed.

Test Plan:
New unit test is introduced
```
yb_build.sh --cxx-test ql-alter-table-test
```

Reviewers: alex, oleg, neil

Reviewed By: oleg, neil

Subscribers: mihnea

Differential Revision: https://phabricator.dev.yugabyte.com/D6990
@d-uspenskiy

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

PHASE 1 completed

@ndeodhar ndeodhar changed the title [YCQL] Cascade ALTER TABLE DROP/RENAME column to index [YCQL] ALTER TABLE DROP/RENAME column should not impact index Aug 28, 2019

@ndeodhar ndeodhar changed the title [YCQL] ALTER TABLE DROP/RENAME column should not impact index [YCQL] ALTER TABLE DROP/RENAME column should not impact index using that column Aug 28, 2019

@ndeodhar

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2019

Tracking phase 2 here: #2176
Closing this issue since phase 1 is complete and we no longer result in an inconsistent DB state.

@ndeodhar ndeodhar closed this Aug 28, 2019

Query Language automation moved this from In progress to Done Aug 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.