Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16330 )

Change subject: KUDU-3181: put a bound on the queue of compilation manager
......................................................................


Patch Set 2:

(3 comments)

Thank you for the patch!

http://gerrit.cloudera.org:8080/#/c/16330/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16330/2//COMMIT_MSG@7
PS2, Line 7: put a bound on the queue of compilation manager
Do you mind adding a test that set the maximum queue size to some very low 
number (say, 1) and runs many concurrent scan requests which would result in 
scheduling a task in the compilation manager's queue?


http://gerrit.cloudera.org:8080/#/c/16330/2/src/kudu/codegen/compilation_manager.cc
File src/kudu/codegen/compilation_manager.cc:

http://gerrit.cloudera.org:8080/#/c/16330/2/src/kudu/codegen/compilation_manager.cc@135
PS2, Line 135: FLAGS_codegen_cache_capacity
What's the reasoning to set the maximum queue size based on the capacity of the 
cache?

I guess I don't see why the capacity of the cache should directly correlate 
with the maximum size of the queue.  Say, what if --codegen_cache_capacity is 
set to 0?

I think it would be cleaner to introduce a separate flag for the maximum size 
of the compilation queue.


http://gerrit.cloudera.org:8080/#/c/16330/2/src/kudu/codegen/compilation_manager.cc@190
PS2, Line 190:     if (!s.ok()) {
             :       VLOG(1) << "RowProjector compilation request submit 
failed";
             :     }
I think WARN_NOT_OK_EVERY_N_SECS would be more appropriate here.



--
To view, visit http://gerrit.cloudera.org:8080/16330
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia85b5818d1a5ea3ea978808900dd9c9e0cc0784e
Gerrit-Change-Number: 16330
Gerrit-PatchSet: 2
Gerrit-Owner: Li Zhiming <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Wed, 12 Aug 2020 17:52:03 +0000
Gerrit-HasComments: Yes

Reply via email to