[GitHub] [rocketmq] StyleTang commented on pull request #3137: [ISSUE #3136] TraceDataEncoder add SubAfter trace bean timestamp

[GitBox <gi...@apache.org>]

StyleTang commented on pull request #3137:
URL: https://github.com/apache/rocketmq/pull/3137#issuecomment-881803349


   @yuz10 @duhenglucky Thanks for your comment.
   
   For subAfterContext, in the previous version, it was already defined line[7] is TimeStamp and line[8] is GroupName, it means for compatible reason in new revision we should still follow this protocol.
   
   The subAfter GroupName is duplicated with subBeofre, so based on performance considerations(I guess), it was removed it by https://github.com/apache/rocketmq/pull/3005/files
   
   
   We have several ways to make it compatible.
   
   ### Option1 ( Prefer )
   
   Just follow the previous version subAfter protocol.
   
   ( line[7] is TimeStamp and line[8] is GroupName )
   
   ##### Props
   
   * The protocol remains consistent, easy to maintain
   
   ##### Cons
   
   * In the future revision should add line[8] group name back, even if it is useless
   
   ### Option2
   
   If line[8] GroupName was never used, just remove it as if it not exist.
   
   ##### Props
   
   * Performance friendly (can remove the duplicated groupName)
   
   ##### Cons
   
   * The protocol is changed (Not a good way)
   * Compatible logic is complicated, old version line[8] is group name while new revision may be not. (The new revision content length should not be 9, if it is 9, we can’t tell what the value it is)
   
   Do you have any suggestions for this?
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org