You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/06/12 11:07:07 UTC

[GitHub] [incubator-doris] jacktengg opened a new pull request, #10080: [improvement] send merged runtime filter asynchrously and improve runtime filter waiting

jacktengg opened a new pull request, #10080:
URL: https://github.com/apache/incubator-doris/pull/10080

   # Proposed changes
   
   Issue Number: close #xxx
   
   ## Problem Summary:
   
   1. Send merged runtime filter asynchrously to improve effeciency.
   For tpch query 3, using 100G data, 1Fe, 2BEs, time for sending merged RF improve from 129 ms to 50 ms.
   
   2. Waiting runtime filters for at most 1 seconds even for multiple runtime filters.
   
   ## Checklist(Required)
   
   1. Does it affect the original behavior: (Yes/No/I Don't know)
   3. Has unit tests been added: (Yes/No/No Need)
   4. Has document been added or modified: (Yes/No/No Need)
   5. Does it need to update dependencies: (Yes/No)
   6. Are there any changes that cannot be rolled back: (Yes/No)
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] github-actions[bot] commented on pull request #10080: [improvement] send merged runtime filter asynchrously

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10080:
URL: https://github.com/apache/incubator-doris/pull/10080#issuecomment-1153838803

   PR approved by at least one committer and no changes requested.


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] xinyiZzz commented on pull request #10080: [improvement] send merged runtime filter asynchrously and improve runtime filter waiting

Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on PR #10080:
URL: https://github.com/apache/incubator-doris/pull/10080#issuecomment-1153168677

   LGTM


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] github-actions[bot] commented on pull request #10080: [improvement] send merged runtime filter asynchrously

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10080:
URL: https://github.com/apache/incubator-doris/pull/10080#issuecomment-1153838828

   PR approved by anyone and no changes requested.


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] xinyiZzz commented on pull request #10080: [improvement] send merged runtime filter asynchrously

Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on PR #10080:
URL: https://github.com/apache/incubator-doris/pull/10080#issuecomment-1153640671

   @jacktengg
   hi, why removed the logic that multiple filters only wait 1s at most. Is there a performance loss in the test?
   
   This is a TODO for the first implementation of the runtime filter. Currently, `ScanNode` will wait at most n seconds, and n is equal to the number of runtime filters, which will cause serious performance loss in extreme cases.


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] xinyiZzz commented on pull request #10080: [improvement] send merged runtime filter asynchrously

Posted by GitBox <gi...@apache.org>.
xinyiZzz commented on PR #10080:
URL: https://github.com/apache/incubator-doris/pull/10080#issuecomment-1153711447

   > > @jacktengg hi, why removed the logic that multiple filters only wait 1s at most. Is there a performance loss in the test?
   > > This is a TODO for the first implementation of the runtime filter. Currently, `ScanNode` will wait at most n seconds, and n is equal to the number of runtime filters, which will cause serious performance loss in extreme cases.
   > 
   > No enough test have been done for the change of 'multiple filters only wait 1s at most', so the performance impact is not sure for mutiple runtime filters in on ScanNode.
   > 
   > E.g., waiting a RF for 1.5 seconds and it can filter a lot of rows, VS waiting for 1 seconds and the RF is missed, I have not tested this situation, not sure which is better.
   > 
   > So in this PR will not include the change of RF waiting.
   
   If waiting for 1 second causes RF to be missed, `runtime_filter_wait_time_ms` should be increased.
   
   Otherwise it will only wait 1.5 seconds if `ScanNode`apply is greater than 1 RF, which is uncontrollable.
   
   `Number of RFs` is not directly related to `Waiting time per RF`.
   
   I think 'multiple filters only wait 1s at most' is a viable optimization, maybe do some testing.


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on pull request #10080: [improvement] send merged runtime filter asynchrously and improve runtime filter waiting

Posted by GitBox <gi...@apache.org>.
morningman commented on PR #10080:
URL: https://github.com/apache/incubator-doris/pull/10080#issuecomment-1153203895

   Hi @jacktengg , please rebase to make p0 test happy.


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] yiguolei commented on pull request #10080: [improvement] send merged runtime filter asynchrously and improve runtime filter waiting

Posted by GitBox <gi...@apache.org>.
yiguolei commented on PR #10080:
URL: https://github.com/apache/incubator-doris/pull/10080#issuecomment-1153361990

   Please add ssb 100g performance test. I am afraid there is performance loss.


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] jacktengg commented on pull request #10080: [improvement] send merged runtime filter asynchrously

Posted by GitBox <gi...@apache.org>.
jacktengg commented on PR #10080:
URL: https://github.com/apache/incubator-doris/pull/10080#issuecomment-1153606495

   > 
   
   
   
   > Please add ssb 100g performance test. I am afraid there is performance loss.
   
   ssb 100G perf test:
   
   
     | master-f26b81e-1replica | Send merged RF asynchronously | Time Ratio
   -- | -- | -- | --
    
   q1.1 | 87 | 87 | 1.00
   q1.2 | 62 | 65.00 | 1.05
   q1.3 | 60 | 64.00 | 1.07
   q2.1 | 529 | 544.00 | 1.03
   q2.2 | 456 | 462.00 | 1.01
   q2.3 | 431 | 437.00 | 1.01
   q3.1 | 1022 | 1055.00 | 1.03
   q3.2 | 463 | 470.00 | 1.02
   q3.3 | 387 | 396.00 | 1.02
   q3.4 | 122 | 120.00 | 0.98
   q4.1 | 1207 | 1221.00 | 1.01
   q4.2 | 757 | 773.00 | 1.02
   q4.3 | 984 | 1001.00 | 1.02
   
   
   No perf lost for ssb.


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] jacktengg commented on pull request #10080: [improvement] send merged runtime filter asynchrously

Posted by GitBox <gi...@apache.org>.
jacktengg commented on PR #10080:
URL: https://github.com/apache/incubator-doris/pull/10080#issuecomment-1153655751

   > @jacktengg hi, why removed the logic that multiple filters only wait 1s at most. Is there a performance loss in the test?
   > 
   > This is a TODO for the first implementation of the runtime filter. Currently, `ScanNode` will wait at most n seconds, and n is equal to the number of runtime filters, which will cause serious performance loss in extreme cases.
   
   No enough test have been done for the change of 'multiple filters only wait 1s at most', so  the performance impact is not sure for mutiple runtime filters in on ScanNode.
   
   E.g., waiting a RF for 1.5 seconds and it can filter a lot of rows, VS waiting for 1 seconds and the RF is missed, I have not tested this situation, not sure which is better.
   
   So in this PR will not include the change of RF waiting.


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] yiguolei merged pull request #10080: [improvement] send merged runtime filter asynchrously

Posted by GitBox <gi...@apache.org>.
yiguolei merged PR #10080:
URL: https://github.com/apache/incubator-doris/pull/10080


-- 
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: commits-unsubscribe@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org