[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/830 @maskit Possibly, but not sure. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user maskit commented on the issue: https://github.com/apache/trafficserver/pull/830 @masaori335 I think this should resolve TS-4618 too, right? https://issues.apache.org/jira/browse/TS-4618 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user maskit commented on the issue: https://github.com/apache/trafficserver/pull/830 Talked with Masaori offline. - Chrome seems creating a deep depth tree intentionally to get contents in the order that Chrome wants - Using SETTINGS_MAX_CONCURRENT_STREAMS as the limit is reasonable because the tree depth doesn't over the setting value even if the tree nodes are created in the Chrome way - There would be no serious issue even if all clients were Chrome So, +1 from me. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/830 FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/508/ for details. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/830 Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/405/ for details. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/830 Yes, we don't have to follow it, but we should follow it as much as possible. Because it could be happen that images are sent before CSS/JS, if we don't follow it. How about use SETTINGS_MAX_CONCURRENT_STREAMS as the max depth? It looks appropriate and we don't need new configures. I'm seeing that with latest Chrome ( 51.0.2704.106 ). Chrome had that bug in Feb and reverted the changes. IIUC, the bug is about the order of streams not depth of streams. Now (maybe from Jun), it looks like they fixed the order and rolled it out. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user maskit commented on the issue: https://github.com/apache/trafficserver/pull/830 My point is that we don't have to follow the crazy dependency request. > Set max depth of Http2DependencyTree When the depth over the maximum, new node will be a children of root node. This means nothing bad happens if we got a 1024 depth tree request, right? By the way, isn't it just a bug on version 51? Does it happen on the latest version? https://bugs.chromium.org/p/chromium/issues/detail?id=590225 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/830 Chrome 51:) The tree of it has all streams in only one branch (all headers frame has exclusive flag). So the depth is equal to steams. For FireFox, 8 - 16 is big enough. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user masaori335 commented on the issue: https://github.com/apache/trafficserver/pull/830 1) 256 seems crazy deep, is that a reasonable default? SETTINGS_MAX_CONCURRENT_STREAMS is 100 at least. And "idle" streams (that are not counted for concurrent streams) can be node of tree. Those can be said for odd-numbered streams which is used by client and even-numbered streams which is used by server. So I chose 256. If we ignore server-push cases, it could be 128. 2) Do we want to make this configurable instead of static? Are there use cases where someone want more (or less) ? Hmm, if someone wants to increase SETTINGS_MAX_CONCURRENT_STREAMS, it looks like this should be increased too. But this is a limit of recursion, so I don't think this should be configurable for now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user maskit commented on the issue: https://github.com/apache/trafficserver/pull/830 Can you add a test for the case that the current depth hit the maximum depth? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/830 Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/386/ for details. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver issue #830: TS-4554: Set max depth of Http2DependencyTree
Github user atsci commented on the issue: https://github.com/apache/trafficserver/pull/830 FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/489/ for details. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
