TS-4612: Proposal: InactivityCop Optimize #771

oknet · 2016-06-30T12:34:07Z

Optimize InactivityCop on a heavy load and high level concurrent connection system.

It is set default inactivity timeout before put vc into open_list.

bryancall · 2016-07-19T08:34:31Z

iocore/net/UnixNetVConnection.cc

  nh->open_list.enqueue(this);

-  ink_assert(!inactivity_timeout_in);
+  // ink_assert(!inactivity_timeout_in);


This can be removed.

zwoop · 2016-07-21T18:29:56Z

@bryancall Should we merge this? [approve ci].

atsci · 2016-07-21T18:33:22Z

Linux build failed! See https://ci.trafficserver.apache.org/job/Github-Linux/367/ for details.

zwoop · 2016-07-21T18:34:12Z

@oknet This fails on the clang-format, can you please fix and push an update?

atsci · 2016-07-21T18:40:37Z

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/470/ for details.

bryancall · 2016-07-22T10:54:07Z

@zwoop Yes, this should be merged after the clang-format.

oknet · 2016-07-23T12:39:51Z

clang-format is done.

zwoop · 2016-07-23T12:56:38Z

[approve ci]

atsci · 2016-07-23T13:08:04Z

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/475/ for details.

atsci · 2016-07-23T13:08:47Z

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/372/ for details.

oknet · 2016-07-23T13:09:18Z

@bryancall I'm found open_list also used in Cluster.

file: iocore/cluster/ClusterHandlerBase.cc

1007     case ClusterHandler::CLCON_CONN_BIND_CLEAR: {
1008       UnixNetVConnection *vc = (UnixNetVConnection *)net_vc;
1009       MUTEX_TRY_LOCK(lock, vc->nh->mutex, e->ethread);
1010       MUTEX_TRY_LOCK(lock1, vc->mutex, e->ethread);
1011       if (lock.is_locked() && lock1.is_locked()) {
1012         vc->ep.stop();
1013         vc->nh->open_list.remove(vc);
1014         vc->thread = NULL;
1015         if (vc->nh->read_ready_list.in(vc))
1016           vc->nh->read_ready_list.remove(vc);
1017         if (vc->nh->write_ready_list.in(vc))
1018           vc->nh->write_ready_list.remove(vc);
1019         if (vc->read.in_enabled_list)
1020           vc->nh->read_enable_list.remove(vc);
1021         if (vc->write.in_enabled_list)
1022           vc->nh->write_enable_list.remove(vc);
1023 
1024         // CLCON_CONN_BIND handle in bind vc->thread (bind thread nh)
1025         cluster_connect_state = ClusterHandler::CLCON_CONN_BIND;
1026         thread->schedule_in(this, CLUSTER_PERIOD);
1027         return EVENT_DONE;
1028       } else {
1029         // CLCON_CONN_BIND_CLEAR handle in origin vc->thread (origin thread nh)
1030         vc->thread->schedule_in(this, CLUSTER_PERIOD);
1031         return EVENT_DONE;
1032       }
1033     }

Is there have InactivityCop for Cluster? I am not familiar with the cluster.
I'm should add "vc->nh->cop_list.remove(vc);" after Line 1013 if yes.

oknet · 2016-07-23T13:25:12Z

update iocore/cluster/ClusterHandlerBase.cc @bryancall @zwoop
but I have no environment to perform cluster system test.

bryancall · 2016-07-26T14:24:27Z

@oknet I don't think it is necessary to remove it from the cop_list, since it is currently not being removed from the list and it is checking the inactivity timeouts. I would leave it as is.

oknet · 2016-07-26T16:26:19Z

@bryancall to review the codes of ClusterHandlerBase.cc

 979         thread = eventProcessor.eventthread[ET_CLUSTER][id % num_of_cluster_threads];
 980         if (net_vc->thread == thread) {
 981           cluster_connect_state = CLCON_CONN_BIND_OK;
 982           break;
 983         } else {
 984           cluster_connect_state = ClusterHandler::CLCON_CONN_BIND_CLEAR;
 985         }

From my understanding of the code, the codes from L1007 to L1033 means:

the net_vc bind to unmatched thread
It should unbind(L1012-1022) and then rebind(L1024-L1027) to another thread

I think the unbind include remove vc from cop_list here.
because the InactivityCop only push the vc->thread == this_ethread() into cop_list, but the vc->thread set to NULL in L1014.

    // Copy the list and use pop() to catch any closes caused by callbacks.
    forl_LL(UnixNetVConnection, vc, nh.open_list)
    {   
      if (vc->thread == this_ethread()) {
        nh.cop_list.push(vc);
      }   
    }

zwoop · 2016-08-18T00:28:54Z

Where are we with this? Ready to land?

atsci · 2016-08-27T11:53:21Z

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/632/ for details.

atsci · 2016-08-27T11:59:26Z

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/528/ for details.

oknet · 2016-08-27T12:07:57Z

rebased on master

jpeach · 2016-09-22T17:54:19Z

@oknet would you mind revising the commit subject and descripton?

TS-4612: Optimize InactivityCop.

Longer description here. Capture the important points from this PR and from the Jira explanation.

oknet · 2016-09-23T16:38:38Z

@jpeach @bryancall add comments and details in commit message

Load the NetVCs from open_list to cop_list before next InactivityCop
runs. NetHandler will remove NetVC from cop_list if it is triggered. As
the NetHandler runs, the number of NetVCs in the cop_list is decreasing.
Lots of NetVCs will be removed from cop_list because the NetHandler runs
100 times maximum between InactivityCop runs. Therefore we don't have to
check all the NetVCs as much as open_list.

atsci · 2016-09-23T16:38:45Z

Linux build failed! See https://ci.trafficserver.apache.org/job/Github-Linux/750/ for details.

atsci · 2016-09-23T16:43:35Z

FreeBSD build failed! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/854/ for details.

atsci · 2016-09-23T16:48:24Z

FreeBSD build failed! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/855/ for details.

atsci · 2016-09-23T16:48:43Z

Linux build failed! See https://ci.trafficserver.apache.org/job/Github-Linux/751/ for details.

Load the NetVCs from open_list to cop_list before next InactivityCop runs. NetHandler will remove NetVC from cop_list if it is triggered. As the NetHandler runs, the number of NetVCs in the cop_list is decreasing. Lots of NetVCs will be removed from cop_list because the NetHandler runs 100 times maximum between InactivityCop runs. Therefore we don't have to check all the NetVCs as much as open_list.

atsci · 2016-09-23T17:39:13Z

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/856/ for details.

atsci · 2016-09-23T17:39:22Z

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/752/ for details.

The h2disable and h2disable_no_accept_threads autests both had curl output expectations that have changed in the recent release of curl. Here is the old output: ``` Using HTTP2, server supports multiplexing ``` Here is the new output ``` using HTTP/2 ``` This updates the ContainsExpression and ExcludesExpression strings to be able to work with both versions of curl. (cherry picked from commit aa33256)

* Normalize Accept-Encoding header value before cache lookup Co-authored-by: Min Chen <mchen@yahoo-corp.jp> * Fix typos and use f-strings for formatting * fix errors in the previous commit * Remove select_ports --------- Co-authored-by: Masakazu Kitajo <maskit@apache.org> Co-authored-by: Min Chen <mchen@yahoo-corp.jp>

oknet changed the title ~~Proposal: InactivityCop Optimize~~ TS-4612: Proposal: InactivityCop Optimize Jun 30, 2016

zwoop added the Core label Jun 30, 2016

zwoop added this to the 7.0.0 milestone Jun 30, 2016

zwoop assigned bryancall Jun 30, 2016

oknet force-pushed the TS-4612 branch from 2ba5b00 to 8cb5b12 Compare July 4, 2016 05:03

bryancall reviewed Jul 19, 2016
View reviewed changes

oknet force-pushed the TS-4612 branch from 8cb5b12 to 7a694c2 Compare July 23, 2016 12:37

oknet force-pushed the TS-4612 branch from 7a694c2 to 29dc6ef Compare July 23, 2016 12:42

oknet force-pushed the TS-4612 branch from 29dc6ef to b3441cf Compare July 23, 2016 13:22

oknet closed this Aug 27, 2016

oknet deleted the TS-4612 branch August 27, 2016 11:35

oknet restored the TS-4612 branch August 27, 2016 11:37

oknet reopened this Aug 27, 2016

oknet force-pushed the TS-4612 branch from b3441cf to 37db36a Compare August 27, 2016 12:07

zwoop modified the milestones: 7.1.0, 7.0.0 Sep 15, 2016

oknet force-pushed the TS-4612 branch from 37db36a to 3446a8f Compare September 23, 2016 16:36

oknet force-pushed the TS-4612 branch from 3446a8f to f49c0cc Compare September 23, 2016 16:42

oknet force-pushed the TS-4612 branch from f49c0cc to 425b696 Compare September 23, 2016 17:24

oknet merged commit 1935d44 into apache:master Sep 23, 2016

masaori335 mentioned this pull request Apr 24, 2017

Fix inactivity_timeout canceling #1716

Merged

oknet mentioned this pull request Apr 24, 2017

Fix for dropped connections due to next_inactivity_timeout_at==0 and inactivity_timeout_in!=0 #1675

Closed

TS-4612: Proposal: InactivityCop Optimize #771

TS-4612: Proposal: InactivityCop Optimize #771

Uh oh!

Conversation

oknet commented Jun 30, 2016

Uh oh!

bryancall Jul 19, 2016

Choose a reason for hiding this comment

Uh oh!

zwoop commented Jul 21, 2016

Uh oh!

atsci commented Jul 21, 2016

Uh oh!

zwoop commented Jul 21, 2016

Uh oh!

atsci commented Jul 21, 2016

Uh oh!

bryancall commented Jul 22, 2016

Uh oh!

oknet commented Jul 23, 2016

Uh oh!

zwoop commented Jul 23, 2016

Uh oh!

atsci commented Jul 23, 2016

Uh oh!

atsci commented Jul 23, 2016

Uh oh!

oknet commented Jul 23, 2016

Uh oh!

oknet commented Jul 23, 2016

Uh oh!

bryancall commented Jul 26, 2016

Uh oh!

oknet commented Jul 26, 2016

Uh oh!

zwoop commented Aug 18, 2016

Uh oh!

atsci commented Aug 27, 2016

Uh oh!

atsci commented Aug 27, 2016

Uh oh!

oknet commented Aug 27, 2016

Uh oh!

jpeach commented Sep 22, 2016

Uh oh!

oknet commented Sep 23, 2016

Uh oh!

atsci commented Sep 23, 2016

Uh oh!

atsci commented Sep 23, 2016

Uh oh!

atsci commented Sep 23, 2016

Uh oh!

atsci commented Sep 23, 2016

Uh oh!

atsci commented Sep 23, 2016

Uh oh!

atsci commented Sep 23, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants