Oved Ourfali has posted comments on this change.

Change subject: engine: DB persistent quartz scheduler
......................................................................


Patch Set 4: Code-Review-1

(2 comments)

Is that all configured by default?
Don't we want to change the setup in order to determine whether to add that or 
not?

It sounds like this doesn't scale. We're keep on improving thread consumption, 
and making the engine scale better.

Also, the amount of DB connections here will increase, which also has 
performance implications.

What's the need that this patch addresses?
Putting -1 until clarifying questions and until reviewed by Yair and Liran as 
well.

http://gerrit.ovirt.org/#/c/36297/4/backend/manager/modules/scheduler/src/main/resources/ovirt-db-scheduler.properties
File 
backend/manager/modules/scheduler/src/main/resources/ovirt-db-scheduler.properties:

Line 2: org.quartz.jobStore.class=org.quartz.impl.jdbcjobstore.JobStoreCMT
Line 3: 
org.quartz.jobStore.driverDelegateClass=org.quartz.impl.jdbcjobstore.StdJDBCDelegate
Line 4: org.quartz.jobStore.dataSource=EngineDS
Line 5: org.quartz.threadPool.class=org.quartz.simpl.SimpleThreadPool
Line 6: org.quartz.threadPool.threadCount=100
what's the use-case here? Do you really need 100 threads for these jobs?
Line 7: org.quartz.jobStore.nonManagedTXDataSource=NMEngineDS
Line 8: org.quartz.dataSource.EngineDS.jndiURL=java:/ENGINEDataSource


http://gerrit.ovirt.org/#/c/36297/4/packaging/services/ovirt-engine/ovirt-engine.xml.in
File packaging/services/ovirt-engine/ovirt-engine.xml.in:

Line 168:           <driver>postgresql</driver>
Line 169:           
<transaction-isolation>TRANSACTION_READ_COMMITTED</transaction-isolation>
Line 170:           <pool>
Line 171:             
<min-pool-size>$getinteger('ENGINE_DB_MIN_CONNECTIONS')</min-pool-size>
Line 172:             
<max-pool-size>$getinteger('ENGINE_DB_MAX_CONNECTIONS')</max-pool-size>
so you're using the same amount of connections as specified for the regular 
data source?
Not sure that's the right approach here.
Line 173:             <prefill>true</prefill>
Line 174:           </pool>
Line 175:           <security>
Line 176:             
<user-name><![CDATA[$getstring('ENGINE_DB_USER')]]></user-name>


-- 
To view, visit http://gerrit.ovirt.org/36297
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a34dac95999cb6b3721d201c116fb5f6089bb61
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sahina Bose <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: anmolbabu <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to