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

fixes(istgt): leaving recvlowat as 1 to read data at earliest #231

Merged
merged 3 commits into from Feb 21, 2019

Conversation

Projects
None yet
3 participants
@vishnuitta
Copy link
Member

commented Feb 21, 2019

This PR picks these PRs (PR#228, 229, 230) from highcpu branch, which includes,

  • Leaving recvlowat as 1 to read data at earliest
  • log for op_text response
  • reading immediate flag from correct bit
  • commented out unused code
  • Not closing the connection on logout request which will be closed only when client triggers it

This PR fixes openebs/openebs#2382, openebs/openebs#2390

Adding automated tests for issue related to #2390 is pending. Adding this automation is making travis run time more than 50 minutes. So, automated tests need to be optimized to complete within 40 mins. It will be taken in another PR.

Signed-off-by: Vitta vitta@mayadata.io

fixes(istgt): leaving recvlowat as 1 to read data at earliest
picking few other PRs from highcpu branch

Signed-off-by: Vitta <vitta@mayadata.io>

@vishnuitta vishnuitta requested review from payes, pawanpraka1 and kmova Feb 21, 2019

@@ -6019,7 +6027,7 @@ worker(void *arg)
conn->exec_lu_task = NULL;
lu_task = NULL;

pthread_cleanup_push(worker_cleanup, conn);
// pthread_cleanup_push(worker_cleanup, conn);

This comment has been minimized.

Copy link
@pawanpraka1

pawanpraka1 Feb 21, 2019

Contributor

do we still need this change? we should call worker_cleanup.

This comment has been minimized.

Copy link
@payes

payes Feb 21, 2019

Member

we don't need it. Since this is being popped out on successful exit of worker thread, and we don't return from between.

(events.events & EPOLLHUP) ||
(!(events.events & EPOLLIN))) {
ISTGT_ERRLOG("close conn %d\n", errno);
if(events.events != EPOLLIN) {

This comment has been minimized.

Copy link
@pawanpraka1

pawanpraka1 Feb 21, 2019

Contributor

in tcp half close, we will get EPOLLRDHUP | EPOLLIN, we should continue in that case.

This comment has been minimized.

Copy link
@vishnuitta

vishnuitta Feb 21, 2019

Author Member

makes sense..

@@ -5343,7 +5351,7 @@ wait_all_task(CONN_Ptr conn)
conn->id, conn->running_tasks);
}


#if f0

This comment has been minimized.

Copy link
@payes

payes Feb 21, 2019

Member

Can we please correct this typo

payes added some commits Feb 21, 2019

@payes

payes approved these changes Feb 21, 2019

@pawanpraka1
Copy link
Contributor

left a comment

looks good.

@pawanpraka1 pawanpraka1 merged commit 39771ce into openebs:v0.8.x Feb 21, 2019

1 of 2 checks passed

Better Code Hub ✋ This code needs to be refactored
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pawanpraka1

This comment has been minimized.

Copy link
Contributor

commented Feb 21, 2019

we need to raise the PR in replication branch also.

vishnuitta added a commit to vishnuitta/istgt that referenced this pull request Feb 21, 2019

fixes(istgt): leaving recvlowat as 1 to read data at earliest (openeb…
…s#231)

- log for op_text response

- reading immediate flag from correct bit

- commented out unused code

- Not closing the connection on logout request which will be closed only when client triggers it

Signed-off-by: Vitta <vitta@mayadata.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.