You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2016/12/03 03:44:14 UTC

[kudu-CR] KUDU-1753 [tablet service] continue scan request on deleted tablet 

Alexey Serbin has uploaded a new change for review.

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

Change subject: KUDU-1753 [tablet_service] continue scan request on deleted tablet
......................................................................

KUDU-1753 [tablet_service] continue scan request on deleted tablet

Updated TabletServiceImpl::HandleContinueScanRequest() to continue
serving in-progress scan requests even if the tablet was deleted
in the middle.

This patch also enables the TestDeleteTableWhileScanInProgress
integration test from the DeleteTableTest suite.

This fixes the following JIRA item:
KUDU-1753 Impala query fails: Unable to advance iterator:
  Illegal state: Tablet is not running

Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
---
M src/kudu/tserver/tablet_service.cc
1 file changed, 27 insertions(+), 15 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] KUDU-1753 [tablet service] continue scan request on deleted tablet

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1753 [tablet_service] continue scan request on deleted tablet
......................................................................


Patch Set 2:

> Alexey, just thinking out loud here:  Tablet not found in the JIRA
 > corresponds to tablet tombstoned state in the tablet server, which
 > means any subsequent scan on that server is not expected to yield
 > any data, right ? This fix aims to continue the scan until it
 > fetches whatever is cached so far. So, I am curious if this
 > guarantees to fix the underlying issue though, because the
 > GetTabletRef can fail anywhere during the scan and not just under
 > HandleContinueScanRequest, right ?

The original issue described in JIRA is 'Illegal state: Tablet is not running'.  The hypothesis about the 'Tablet not found'  -- is just a hypothesis that it's related to that.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-1753 [tablet service] continue scan request on deleted tablet

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1753 [tablet_service] continue scan request on deleted tablet
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5346/2/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

PS2, Line 1687: if (PREDICT_FALSE(!(s.ok() || s.IsIllegalState()))) {
> PREDICT_FALSE(!s.ok() && !s.IsIllegalState()) is slightly more readable (do
Well, the readability thing is reversed to me :)  I find current version easier to comprehend.  But fine -- I'll update it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1753 [tablet service] continue scan request on deleted tablet

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1753 [tablet_service] continue scan request on deleted tablet
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5346/2/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

PS2, Line 1687: if (PREDICT_FALSE(!(s.ok() || s.IsIllegalState()))) {
PREDICT_FALSE(!s.ok() && !s.IsIllegalState()) is slightly more readable (don't have to mentally apply the negation) also maybe move the comment above about availability here?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1753 continue scan if tablet is being deleted

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1753 continue scan if tablet is being deleted
......................................................................

KUDU-1753 continue scan if tablet is being deleted

Updated TabletServiceImpl::HandleContinueScanRequest() to continue
serving in-progress scan requests even if the tablet was deleted
in the middle.

This patch also enables the TestDeleteTableWhileScanInProgress
integration test from the DeleteTableTest suite.

This is intended to fix the following JIRA item:
KUDU-1753 Impala query fails: Unable to advance iterator:
  Illegal state: Tablet is not running

Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
---
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 28 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/5346/6
-- 
To view, visit http://gerrit.cloudera.org:8080/5346
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-1753 [tablet service] continue scan request on deleted tablet

Posted by "Dinesh Bhat (Code Review)" <ge...@cloudera.org>.
Dinesh Bhat has posted comments on this change.

Change subject: KUDU-1753 [tablet_service] continue scan request on deleted tablet
......................................................................


Patch Set 5:

> > Alexey, just thinking out loud here:  Tablet not found in the
 > JIRA
 > > corresponds to tablet tombstoned state in the tablet server,
 > which
 > > means any subsequent scan on that server is not expected to yield
 > > any data, right ? This fix aims to continue the scan until it
 > > fetches whatever is cached so far. So, I am curious if this
 > > guarantees to fix the underlying issue though, because the
 > > GetTabletRef can fail anywhere during the scan and not just under
 > > HandleContinueScanRequest, right ?
 > 
 > The original issue described in JIRA is 'Illegal state: Tablet is
 > not running'.  The hypothesis about the 'Tablet not found'  -- is
 > just a hypothesis that it's related to that.

I discovered recently that TABLET_NOT_RUNNING error code embodies tablets in bootstrapped state, and also the tablets failed to come up on the node for whatever reasons. There is also another distinct error code TABLET_NOT_FOUND for deleted tablets and this fix is aimed to address the latter situation, right ? The description in the JIRA seemed to indicate this situation may arise due to former error code too. I am just curious if the fix and the related test have taken former error code into account.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-1753 [tablet service] continue scan request on deleted tablet

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1753 [tablet_service] continue scan request on deleted tablet
......................................................................


Patch Set 5:

> Did you make sure the test is not flaky anymore?

I didn't recall any flakiness with the test I added.  What do you mean?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-1753 continue scan if tablet is being deleted

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1753 continue scan if tablet is being deleted
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5346/6/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

PS6, Line 1686: if (PREDICT_FALSE(!s.ok() && !s.IsIllegalState())) {
> how about you make this more explicit:
good idea, thanks


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1753 continue scan if tablet is being deleted

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged.

Change subject: KUDU-1753 continue scan if tablet is being deleted
......................................................................


KUDU-1753 continue scan if tablet is being deleted

Updated TabletServiceImpl::HandleContinueScanRequest() to continue
serving in-progress scan requests even if the tablet was deleted
in the middle.

This patch also enables the TestDeleteTableWhileScanInProgress
integration test from the DeleteTableTest suite.

This is intended to fix the following JIRA item:
KUDU-1753 Impala query fails: Unable to advance iterator:
  Illegal state: Tablet is not running

Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Reviewed-on: http://gerrit.cloudera.org:8080/5346
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 30 insertions(+), 16 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-1753 [tablet service] continue scan request on deleted tablet

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1753 [tablet_service] continue scan request on deleted tablet
......................................................................


Patch Set 5:

> > > Alexey, just thinking out loud here:  Tablet not found in the
 > > JIRA
 > > > corresponds to tablet tombstoned state in the tablet server,
 > > which
 > > > means any subsequent scan on that server is not expected to
 > yield
 > > > any data, right ? This fix aims to continue the scan until it
 > > > fetches whatever is cached so far. So, I am curious if this
 > > > guarantees to fix the underlying issue though, because the
 > > > GetTabletRef can fail anywhere during the scan and not just
 > under
 > > > HandleContinueScanRequest, right ?
 > >
 > > The original issue described in JIRA is 'Illegal state: Tablet is
 > > not running'.  The hypothesis about the 'Tablet not found'  -- is
 > > just a hypothesis that it's related to that.
 > 
 > I discovered recently that TABLET_NOT_RUNNING error code embodies
 > tablets in bootstrapped state, and also the tablets failed to come
 > up on the node for whatever reasons. There is also another distinct
 > error code TABLET_NOT_FOUND for deleted tablets and this fix is
 > aimed to address the latter situation, right ? The description in
 > the JIRA seemed to indicate this situation may arise due to former
 > error code too. I am just curious if the fix and the related test
 > have taken former error code into account.

Yep, it might happen that the tablet is not running not due to the fact it is deleted in the middle, but due to the other reasons.

This fix and the test addresses the case of when tablet is deleted while the scan operation was in progress.  That's what I saw from logs I collected in Impala cluster.  And the test shows that prior to that fix that situation was not handled properly.

I don't think this could address the problem related to TABLET_NOT_FOUND or other situation when TABLET_IS_NOT_RUNNING returned, and this fix is not intended to fix that.  The scope of this fix and test is to deal with issues related to HandleContinueScanRequest().  The short description might be confusing, but more detailed description of explains that.

I will change the short description to

'continue scan on tablet being deleted'

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-1753 [tablet service] continue scan request on deleted tablet

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1753 [tablet_service] continue scan request on deleted tablet
......................................................................

KUDU-1753 [tablet_service] continue scan request on deleted tablet

Updated TabletServiceImpl::HandleContinueScanRequest() to continue
serving in-progress scan requests even if the tablet was deleted
in the middle.

This patch also enables the TestDeleteTableWhileScanInProgress
integration test from the DeleteTableTest suite.

This fixes the following JIRA item:
KUDU-1753 Impala query fails: Unable to advance iterator:
  Illegal state: Tablet is not running

Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
---
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 28 insertions(+), 16 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-1753 [tablet service] continue scan request on deleted tablet

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1753 [tablet_service] continue scan request on deleted tablet
......................................................................


Patch Set 5:

Did you make sure the test is not flaky anymore?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-1753 continue scan if tablet is being deleted

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: KUDU-1753 continue scan if tablet is being deleted
......................................................................


Patch Set 8: Code-Review+2

Re-based the patch, David already gave +2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-1753 continue scan if tablet is being deleted

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1753 continue scan if tablet is being deleted
......................................................................


Patch Set 6:

(2 comments)

lgtm, excepting nits. see my comment on the test patch though.

http://gerrit.cloudera.org:8080/#/c/5346/6/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

PS6, Line 1686: PREDICT_FALSE(!s.ok() && !s.IsIllegalState())
nit: you only need to check !s.IsIllegalState()


PS6, Line 1686: if (PREDICT_FALSE(!s.ok() && !s.IsIllegalState())) {
how about you make this more explicit:

if (PREDICT_FALSE(!s.ok() || tablet_ref_error_code != TABLET_NOT_RUNNING) ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1753 [tablet service] continue scan request on deleted tablet

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1753 [tablet_service] continue scan request on deleted tablet
......................................................................


Patch Set 5:

ha, my bad. Thought this was an old test that was disabled due to flakyness and that you were enabling.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-1753 continue scan if tablet is being deleted

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1753 continue scan if tablet is being deleted
......................................................................

KUDU-1753 continue scan if tablet is being deleted

Updated TabletServiceImpl::HandleContinueScanRequest() to continue
serving in-progress scan requests even if the tablet was deleted
in the middle.

This patch also enables the TestDeleteTableWhileScanInProgress
integration test from the DeleteTableTest suite.

This is intended to fix the following JIRA item:
KUDU-1753 Impala query fails: Unable to advance iterator:
  Illegal state: Tablet is not running

Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
---
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 30 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/5346/7
-- 
To view, visit http://gerrit.cloudera.org:8080/5346
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-1753 [tablet service] continue scan request on deleted tablet

Posted by "Dinesh Bhat (Code Review)" <ge...@cloudera.org>.
Dinesh Bhat has posted comments on this change.

Change subject: KUDU-1753 [tablet_service] continue scan request on deleted tablet
......................................................................


Patch Set 2:

Alexey, just thinking out loud here:  Tablet not found in the JIRA corresponds to tablet tombstoned state in the tablet server, which means any subsequent scan on that server is not expected to yield any data, right ? This fix aims to continue the scan until it fetches whatever is cached so far. So, I am curious if this guarantees to fix the underlying issue though, because the GetTabletRef can fail anywhere during the scan and not just under HandleContinueScanRequest, right ?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-1753 [tablet service] continue scan request on deleted tablet

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1753 [tablet_service] continue scan request on deleted tablet
......................................................................

KUDU-1753 [tablet_service] continue scan request on deleted tablet

Updated TabletServiceImpl::HandleContinueScanRequest() to continue
serving in-progress scan requests even if the tablet was deleted
in the middle.

This patch also enables the TestDeleteTableWhileScanInProgress
integration test from the DeleteTableTest suite.

This fixes the following JIRA item:
KUDU-1753 Impala query fails: Unable to advance iterator:
  Illegal state: Tablet is not running

Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
---
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/tserver/tablet_service.cc
2 files changed, 28 insertions(+), 16 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1753 continue scan if tablet is being deleted

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1753 continue scan if tablet is being deleted
......................................................................


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica48c52a81862f47a9245003915d18be411bf8b1
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <di...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No