You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "XiaokaiWang (Code Review)" <ge...@cloudera.org> on 2019/07/02 07:20:53 UTC

[kudu-CR] alter schema transaction-ToString function error condition is reverse, when error happens the server may crash 

XiaokaiWang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13782


Change subject: alter_schema_transaction-ToString function error_ condition is reverse, when error happens the server may crash
......................................................................

alter_schema_transaction-ToString function error_ condition is reverse, when error happens the server may crash

Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
---
M src/kudu/tablet/transactions/alter_schema_transaction.cc
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/13782/1
-- 
To view, visit http://gerrit.cloudera.org:8080/13782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
Gerrit-Change-Number: 13782
Gerrit-PatchSet: 1
Gerrit-Owner: XiaokaiWang <xi...@live.com>

[kudu-CR] [tserver] Fix bug in AlterSchemaTransactionState::ToString

Posted by "XiaokaiWang (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Yao Xu, Grant Henke, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13782

to look at the new patch set (#2).

Change subject: [tserver] Fix bug in AlterSchemaTransactionState::ToString
......................................................................

[tserver] Fix bug in AlterSchemaTransactionState::ToString

The error_ condition is reversed, When error is null the tserver will crash.

stacktrace:
0x21f5dcc google::protobuf::TextFormat::Printer::Print()
0x21f5e6d google::protobuf::TextFormat::Printer::PrintToString()
0x204e335 kudu::pb_util::SecureShortDebugString()
0xbeae76 kudu::tablet::AlterSchemaTransactionState::ToString()
0xbead3f kudu::tablet::AlterSchemaTransaction::ToString()
0xbedaea kudu::tablet::TransactionDriver::ToString()
0xbf41e1 kudu::tablet::TransactionTracker::WaitForAllToFinish()
0xbf496f kudu::tablet::TransactionTracker::WaitForAllToFinish()
0xbe543f kudu::tablet::TabletReplica::Stop()
0x9cd198 kudu::tserver::TSTabletManager::DeleteTablet()
0x9d2f5f kudu::tserver::DeleteTabletRunnable::Run()
0x207365f kudu::ThreadPool::DispatchThread()
0x2068ec4 kudu::Thread::SuperviseThread()
0x7f01548b2dc5 start_thread
0x7f0152b8dced __clone

Deleting old range partion, which will wait for all txns to finish.
If being dumped txns contains AlterSchema TXN and the txn is executed correct,
tserver will crach.

Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
---
M src/kudu/tablet/transactions/alter_schema_transaction.cc
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/13782/2
-- 
To view, visit http://gerrit.cloudera.org:8080/13782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
Gerrit-Change-Number: 13782
Gerrit-PatchSet: 2
Gerrit-Owner: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>

[kudu-CR] [tserver] Fix bug in AlterSchemaTransactionState::ToString

Posted by "XiaokaiWang (Code Review)" <ge...@cloudera.org>.
XiaokaiWang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13782 )

Change subject: [tserver] Fix bug in AlterSchemaTransactionState::ToString
......................................................................


Patch Set 2:

> Build Failed
 > 
 > http://jenkins.kudu.apache.org/job/kudu-gerrit/18177/ : FAILURE

I look up the error, which is no relation with the cr.


-- 
To view, visit http://gerrit.cloudera.org:8080/13782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
Gerrit-Change-Number: 13782
Gerrit-PatchSet: 2
Gerrit-Owner: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 13:15:54 +0000
Gerrit-HasComments: No

[kudu-CR] [tserver] Fix bug in AlterSchemaTransactionState::ToString

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13782 )

Change subject: [tserver] Fix bug in AlterSchemaTransactionState::ToString
......................................................................

[tserver] Fix bug in AlterSchemaTransactionState::ToString

The error_ condition is reversed, When error is null the tserver will crash.

Stacktrace:
0x21f5dcc google::protobuf::TextFormat::Printer::Print()
0x21f5e6d google::protobuf::TextFormat::Printer::PrintToString()
0x204e335 kudu::pb_util::SecureShortDebugString()
0xbeae76 kudu::tablet::AlterSchemaTransactionState::ToString()
0xbead3f kudu::tablet::AlterSchemaTransaction::ToString()
0xbedaea kudu::tablet::TransactionDriver::ToString()
0xbf41e1 kudu::tablet::TransactionTracker::WaitForAllToFinish()
0xbf496f kudu::tablet::TransactionTracker::WaitForAllToFinish()
0xbe543f kudu::tablet::TabletReplica::Stop()
0x9cd198 kudu::tserver::TSTabletManager::DeleteTablet()
0x9d2f5f kudu::tserver::DeleteTabletRunnable::Run()
0x207365f kudu::ThreadPool::DispatchThread()
0x2068ec4 kudu::Thread::SuperviseThread()
0x7f01548b2dc5 start_thread
0x7f0152b8dced __clone

Deleting old range partion, which will wait for all txns to finish.
If being dumped txns contains 'AlterSchema-TXN', the tserver will crash.

Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
Reviewed-on: http://gerrit.cloudera.org:8080/13782
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Reviewed-by: Grant Henke <gr...@apache.org>
---
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/alter_schema_transaction.h
2 files changed, 2 insertions(+), 1 deletion(-)

Approvals:
  Kudu Jenkins: Verified
  Andrew Wong: Looks good to me, approved
  Grant Henke: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/13782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
Gerrit-Change-Number: 13782
Gerrit-PatchSet: 4
Gerrit-Owner: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>

[kudu-CR] alter schema transaction-ToString function error condition is reverse, when error happens the server may crash

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13782 )

Change subject: alter_schema_transaction-ToString function error_ condition is reverse, when error happens the server may crash
......................................................................


Patch Set 1:

(1 comment)

Thanks for catching this!

http://gerrit.cloudera.org:8080/#/c/13782/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13782/1//COMMIT_MSG@7
PS1, Line 7: alter_schema_transaction-ToString function error_ condition is reverse, when error happens the server may crash
nit: Can you split this into a title and a short body with a little bit of context?

Maybe even include the stacktrace you saw upon failure? How did you see this fail?



-- 
To view, visit http://gerrit.cloudera.org:8080/13782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
Gerrit-Change-Number: 13782
Gerrit-PatchSet: 1
Gerrit-Owner: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 08:03:39 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tserver] Fix bug in AlterSchemaTransactionState::ToString

Posted by "XiaokaiWang (Code Review)" <ge...@cloudera.org>.
XiaokaiWang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13782 )

Change subject: [tserver] Fix bug in AlterSchemaTransactionState::ToString
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13782/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13782/1//COMMIT_MSG@7
PS1, Line 7: [tserver] Fix bug in AlterSchemaTransactionState::ToString
> nit: Can you split this into a title and a short body with a little bit of 
stacktrace:
               0x21f5dcc google::protobuf::TextFormat::Printer::Print()
               0x21f5e6d google::protobuf::TextFormat::Printer::PrintToString()
               0x204e335 kudu::pb_util::SecureShortDebugString()
               0xbeae76 kudu::tablet::AlterSchemaTransactionState::ToString()
               0xbead3f kudu::tablet::AlterSchemaTransaction::ToString()
               0xbedaea kudu::tablet::TransactionDriver::ToString()
               0xbf41e1 kudu::tablet::TransactionTracker::WaitForAllToFinish()
               0xbf496f kudu::tablet::TransactionTracker::WaitForAllToFinish()
               0xbe543f kudu::tablet::TabletReplica::Stop()
               0x9cd198 kudu::tserver::TSTabletManager::DeleteTablet()
               0x9d2f5f kudu::tserver::DeleteTabletRunnable::Run()
               0x207365f kudu::ThreadPool::DispatchThread()
               0x2068ec4 kudu::Thread::SuperviseThread()
               0x7f01548b2dc5 start_thread
               0x7f0152b8dced __clone

In our environment, everyday we will delete old range partitions(three days ago). May it has much transactions and AlterSchema txn cost more than 1s, when dumping txns to string, the tserver crashed.


http://gerrit.cloudera.org:8080/#/c/13782/1//COMMIT_MSG@7
PS1, Line 7: [tserver] Fix bug in AlterSchemaTransactionState::ToString
> Need to wrap. Btw, I think the headline should be more concise.
OK, thanks.



-- 
To view, visit http://gerrit.cloudera.org:8080/13782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
Gerrit-Change-Number: 13782
Gerrit-PatchSet: 2
Gerrit-Owner: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 09:51:11 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tserver] Fix bug in AlterSchemaTransactionState::ToString

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13782 )

Change subject: [tserver] Fix bug in AlterSchemaTransactionState::ToString
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/13782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
Gerrit-Change-Number: 13782
Gerrit-PatchSet: 3
Gerrit-Owner: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 15:51:52 +0000
Gerrit-HasComments: No

[kudu-CR] alter schema transaction-ToString function error condition is reverse, when error happens the server may crash

Posted by "Yao Xu (Code Review)" <ge...@cloudera.org>.
Yao Xu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13782 )

Change subject: alter_schema_transaction-ToString function error_ condition is reverse, when error happens the server may crash
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13782/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13782/1//COMMIT_MSG@7
PS1, Line 7: alter_schema_transaction-ToString function error_ condition is reverse, when error happens the server may crash
Need to wrap. Btw, I think the headline should be more concise.
You can write like this: 

[tserver] Fix bug in AlterSchemaTransactionState::ToString

balabalabala....



-- 
To view, visit http://gerrit.cloudera.org:8080/13782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
Gerrit-Change-Number: 13782
Gerrit-PatchSet: 1
Gerrit-Owner: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 08:00:52 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tserver] Fix bug in AlterSchemaTransactionState::ToString

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13782 )

Change subject: [tserver] Fix bug in AlterSchemaTransactionState::ToString
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/13782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
Gerrit-Change-Number: 13782
Gerrit-PatchSet: 3
Gerrit-Owner: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 15:54:58 +0000
Gerrit-HasComments: No

[kudu-CR] [tserver] Fix bug in AlterSchemaTransactionState::ToString

Posted by "XiaokaiWang (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Yao Xu, Grant Henke, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13782

to look at the new patch set (#3).

Change subject: [tserver] Fix bug in AlterSchemaTransactionState::ToString
......................................................................

[tserver] Fix bug in AlterSchemaTransactionState::ToString

The error_ condition is reversed, When error is null the tserver will crash.

Stacktrace:
0x21f5dcc google::protobuf::TextFormat::Printer::Print()
0x21f5e6d google::protobuf::TextFormat::Printer::PrintToString()
0x204e335 kudu::pb_util::SecureShortDebugString()
0xbeae76 kudu::tablet::AlterSchemaTransactionState::ToString()
0xbead3f kudu::tablet::AlterSchemaTransaction::ToString()
0xbedaea kudu::tablet::TransactionDriver::ToString()
0xbf41e1 kudu::tablet::TransactionTracker::WaitForAllToFinish()
0xbf496f kudu::tablet::TransactionTracker::WaitForAllToFinish()
0xbe543f kudu::tablet::TabletReplica::Stop()
0x9cd198 kudu::tserver::TSTabletManager::DeleteTablet()
0x9d2f5f kudu::tserver::DeleteTabletRunnable::Run()
0x207365f kudu::ThreadPool::DispatchThread()
0x2068ec4 kudu::Thread::SuperviseThread()
0x7f01548b2dc5 start_thread
0x7f0152b8dced __clone

Deleting old range partion, which will wait for all txns to finish.
If being dumped txns contains 'AlterSchema-TXN', the tserver will crash.

Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
---
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/alter_schema_transaction.h
2 files changed, 2 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/13782/3
-- 
To view, visit http://gerrit.cloudera.org:8080/13782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7126edcdddae9cf21d343e6c6c219a003edb1be1
Gerrit-Change-Number: 13782
Gerrit-PatchSet: 3
Gerrit-Owner: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: XiaokaiWang <xi...@live.com>
Gerrit-Reviewer: Yao Xu <oc...@gmail.com>