Github user masaori335 commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/632#discussion_r63741679
  
    --- Diff: proxy/http2/Http2ConnectionState.cc ---
    @@ -906,74 +957,140 @@ 
Http2ConnectionState::update_initial_rwnd(Http2WindowSize new_size)
     }
     
     void
    -Http2ConnectionState::send_data_frame(Http2Stream *stream)
    +Http2ConnectionState::schedule_stream(Http2Stream *stream)
     {
    -  size_t buf_len = 
BUFFER_SIZE_FOR_INDEX(buffer_size_index[HTTP2_FRAME_TYPE_DATA]) - 
HTTP2_FRAME_HEADER_LEN;
    -  uint8_t payload_buffer[buf_len];
    +  DebugHttp2Stream(ua_session, stream->get_id(), "Scheduled");
     
    -  if (stream->get_state() == HTTP2_STREAM_STATE_CLOSED) {
    +  DependencyTree::Node *node = stream->priority_node;
    +  ink_release_assert(node != NULL);
    +
    +  SCOPED_MUTEX_LOCK(lock, this->mutex, this_ethread());
    +  dependency_tree->activate(node);
    +
    +  if (!_scheduled) {
    +    _scheduled = true;
    +
    +    SET_HANDLER(&Http2ConnectionState::main_event_handler);
    +    this_ethread()->schedule_imm_local((Continuation *)this, 
HTTP2_SESSION_EVENT_XMIT);
    +  }
    +}
    +
    +void
    +Http2ConnectionState::send_data_frames_depends_on_priority()
    +{
    +  DependencyTree::Node *node = dependency_tree->top();
    +
    +  // No node to send or no connection level window left
    +  if (node == NULL || client_rwnd <= 0) {
         return;
       }
     
    -  for (;;) {
    -    uint8_t flags = 0x00;
    +  Http2Stream *stream = node->t;
    +  ink_release_assert(stream != NULL);
    +  DebugHttp2Stream(ua_session, stream->get_id(), "top node, point=%d", 
node->point);
     
    -    size_t window_size = min(this->client_rwnd, stream->client_rwnd);
    -    size_t send_size = min(buf_len, window_size);
    -    size_t payload_length;
    -    IOBufferReader *current_reader = stream->response_get_data_reader();
    +  size_t len = 0;
    +  Http2SendADataFrameResult result = send_a_data_frame(stream, len);
     
    -    // Are we at the end?
    -    // If we break here, we never send the END_STREAM in the case of a
    -    // early terminating OS.  Ok if there is no body yet.  Otherwise
    -    // continue on to delete the stream
    -    if (stream->is_body_done() && current_reader && 
!current_reader->is_read_avail_more_than(0)) {
    -      Debug("http2_con", "End of Stream id=%d no more data and body done", 
stream->get_id());
    -      flags |= HTTP2_FLAGS_DATA_END_STREAM;
    -      payload_length = 0;
    -    } else {
    -      // Select appropriate payload size
    -      if (this->client_rwnd <= 0 || stream->client_rwnd <= 0)
    -        break;
    -      // Copy into the payload buffer.  Seems like we should be able to 
skip this
    -      // copy step
    -      payload_length = current_reader ? 
current_reader->read(payload_buffer, send_size) : 0;
    -
    -      if (payload_length == 0 && !stream->is_body_done()) {
    -        break;
    -      }
    +  if (result != HTTP2_SEND_A_DATA_FRAME_NO_ERROR) {
    +    // When no stream level window left, deactivate node once and wait 
window_update frame
    +    dependency_tree->deactivate(node, len);
    +    this_ethread()->schedule_imm_local((Continuation *)this, 
HTTP2_SESSION_EVENT_XMIT);
    +    return;
    +  }
     
    -      // Update window size
    -      this->client_rwnd -= payload_length;
    -      stream->client_rwnd -= payload_length;
    +  // No response body to send
    +  if (len == 0 && !stream->is_body_done()) {
    +    dependency_tree->deactivate(node, len);
    +    this_ethread()->schedule_imm_local((Continuation *)this, 
HTTP2_SESSION_EVENT_XMIT);
    +    return;
    +  }
     
    -      if (stream->is_body_done() && payload_length < send_size) {
    -        flags |= HTTP2_FLAGS_DATA_END_STREAM;
    -      }
    +  if (stream->get_state() == HTTP2_STREAM_STATE_CLOSED) {
    +    dependency_tree->deactivate(node, len);
    +    delete_stream(stream);
    --- End diff --
    
    IMO, we need schedule the continuation again. Because it could be possible 
that another stream in the dependency tree is ready to send.


---
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.
---

Reply via email to