Re: [Engine-devel] Opimizing Postgres Stored Procedures

From: "Laszlo Hornyak" <lhornyak(a)redhat.com&gt; To: "Eli Mesika" <emesika(a)redhat.com&gt; Cc: "engine-devel" <engine-devel(a)ovirt.org&gt; Sent: Sunday, September 1, 2013 5:13:39 PM Subject: Re: [Engine-devel] Opimizing Postgres Stored Procedures ----- Original Message ----- > From: "Eli Mesika" <emesika(a)redhat.com&gt; > To: "Laszlo Hornyak" <lhornyak(a)redhat.com&gt; > Cc: "engine-devel" <engine-devel(a)ovirt.org&gt; > Sent: Sunday, September 1, 2013 3:29:06 PM > Subject: Re: [Engine-devel] Opimizing Postgres Stored Procedures > > > > ----- Original Message ----- > > From: "Laszlo Hornyak" <lhornyak(a)redhat.com&gt; > > To: "Eli Mesika" <emesika(a)redhat.com&gt; > > Cc: "engine-devel" <engine-devel(a)ovirt.org&gt; > > Sent: Sunday, September 1, 2013 2:47:02 PM > > Subject: Re: [Engine-devel] Opimizing Postgres Stored Procedures > > > > > > > > ----- Original Message ----- > > > From: "Eli Mesika" <emesika(a)redhat.com&gt; > > > To: "Laszlo Hornyak" <lhornyak(a)redhat.com&gt; > > > Cc: "engine-devel" <engine-devel(a)ovirt.org&gt; > > > Sent: Sunday, September 1, 2013 10:35:43 AM > > > Subject: Re: [Engine-devel] Opimizing Postgres Stored Procedures > > > > > > > > > > > > ----- Original Message ----- > > > > From: "Laszlo Hornyak" <lhornyak(a)redhat.com&gt; > > > > To: "Eli Mesika" <emesika(a)redhat.com&gt; > > > > Cc: "engine-devel" <engine-devel(a)ovirt.org&gt; > > > > Sent: Friday, August 30, 2013 7:17:32 PM > > > > Subject: Re: [Engine-devel] Opimizing Postgres Stored Procedures > > > > > > > > Hi Eli, > > > > > > > > I wrote a quick benchmark to see if there is any difference when > > > > using > > > > STABLE > > > > modifier on functions running queries the way the engine does it > > > > (calling > > > > it > > > > from JDBC, one function in a single statement) > > > > > > > > with a stable function: > > > > create function getKakukk(_id int) returns VARCHAR STABLE as 'select > > > > val > > > > from > > > > kakukk where id = $1' language sql; > > > > and one not marked as stable > > > > create function getKakukk_(_id int) returns VARCHAR as 'select val > > > > from > > > > kakukk where id = $1' language sql; > > > > the table is this simple: > > > > create table kakukk(id int primary key, val varchar); > > > > and the only content is: > > > > insert into kakukk (id, val) values (1, 'bla bla bla'); > > > > > > > > Now the benchmark code: > > > > > > > > package com.foobar; > > > > > > > > import java.sql.Connection; > > > > import java.sql.DriverManager; > > > > import java.sql.PreparedStatement; > > > > import java.sql.ResultSet; > > > > import java.sql.SQLException; > > > > > > > > import org.junit.After; > > > > import org.junit.Before; > > > > import org.junit.Test; > > > > > > > > public class SpeedTest { > > > > > > > > Connection connection; > > > > > > > > @Before > > > > public void connect() throws SQLException { > > > > connection = > > > > DriverManager.getConnection("jdbc:postgresql://localhost/stabletest", > > > > "engine", "engine"); > > > > } > > > > > > > > @After > > > > public void disconnect() throws SQLException { > > > > connection.close(); > > > > } > > > > > > > > private long measure(Runnable runnable, int times) { > > > > final long start = System.currentTimeMillis(); > > > > for (int i = 0; i < times; i++) { > > > > runnable.run(); > > > > } > > > > final long end = System.currentTimeMillis(); > > > > return end - start; > > > > } > > > > > > > > public static class Select implements Runnable { > > > > > > > > public Select(PreparedStatement preparedStatement) { > > > > super(); > > > > this.preparedStatement = preparedStatement; > > > > } > > > > > > > > final PreparedStatement preparedStatement; > > > > > > > > public void run() { > > > > try ( > > > > ResultSet resultSet = > > > > preparedStatement.executeQuery();) > > > > { > > > > while (resultSet.next()) { > > > > // nothing, just next > > > > } > > > > } catch (SQLException e) { > > > > // TODO Auto-generated catch block > > > > e.printStackTrace(); > > > > } > > > > } > > > > } > > > > > > > > @Test > > > > public void performTest() throws SQLException { > > > > for (int i = 0; i < 10; i++) { > > > > try ( > > > > PreparedStatement stable = > > > > connection.prepareStatement("select > > > > getKakukk(1)"); > > > > PreparedStatement notStable = > > > > connection.prepareStatement("select > > > > getKakukk_(1)");) > > > > { > > > > System.out.println("STABLE: " + measure(new > > > > Select(stable), > > > > 100000)); > > > > System.out.println("not STABLE: " + measure(new > > > > Select(notStable), 100000)); > > > > System.out.println("---"); > > > > > > > > } > > > > } > > > > } > > > > } > > > > > > > > > > > > -- > > > > > > > > The results are very similar, seemingly no difference at all. > > > > Therefore, it seems we do not need those STABLE markers for > > > > performance > > > > reasons. > > > > > > Please refer to > > > http://www.postgresql.org/docs/8.3/static/xfunc-volatility.html > > > It says : > > > > > > "For best optimization results, you should label your functions with > > > the > > > strictest volatility category that is valid for them." > > > > I am sure the postgres guys did not mean "without even thinking about it" > > :) > > The way the enginge is using these functions, it will not improve > > performance, which is not a problem. The problem in my opinion is that > > more > > requirements increase the time needed for review procedure, which is > > already > > quite lengthy. This new requirement does not seem to have any benefit. If > > we > > want to improve the development process then we should not have > > requirements > > that do not come with benefits. > > I totally disagree, I think the opposite , if this will not be a clear rule > of thumb , it will be used incorrectly. > Thinking on each SP if you will benefit or not in terms of performance if > you use those keywords will increase the review time when any reviewer will > need to think if he needs that or not. > I don't think that adding a word to the SP will affect development or > review > time , it should be like you are writing the word FUNCTION for a SP > definition and will become a habit. > I am against complicated rules like "add those keywords whenever you like" > , No, it is even more simple than that: do not add those keywords, it would only make someone believe it will improve database performance, while it clearly won't :)

> if the documentations says a clear instruction to use those keywords in > certain circumstances, that's what I would exactly do and recommend to > others. > > Thanks > > > > > > Best regards, > > Laszlo > > > > > > > > So , using STABLE , IMMUTABLE , STRICT is mandatory from now on. > > > > > > Thanks > > > Eli > > > > > > > > > > > > > > Thank you, > > > > Laszlo > > > > > > > > ----- Original Message ----- > > > > > From: "Laszlo Hornyak" <lhornyak(a)redhat.com&gt; > > > > > To: "Eli Mesika" <emesika(a)redhat.com&gt; > > > > > Cc: "engine-devel" <engine-devel(a)ovirt.org&gt; > > > > > Sent: Wednesday, August 28, 2013 1:02:18 PM > > > > > Subject: Re: [Engine-devel] Opimizing Postgres Stored Procedures > > > > > > > > > > > > > > > ----- Original Message ----- > > > > > > From: "Eli Mesika" <emesika(a)redhat.com&gt; > > > > > > To: "Laszlo Hornyak" <lhornyak(a)redhat.com&gt; > > > > > > Cc: "engine-devel" <engine-devel(a)ovirt.org&gt; > > > > > > Sent: Wednesday, August 28, 2013 11:45:14 AM > > > > > > Subject: Re: [Engine-devel] Opimizing Postgres Stored Procedures > > > > > > > > > > > > > > > > > > > > > > > > ----- Original Message ----- > > > > > > > From: "Laszlo Hornyak" <lhornyak(a)redhat.com&gt; > > > > > > > To: "Eli Mesika" <emesika(a)redhat.com&gt; > > > > > > > Cc: "engine-devel" <engine-devel(a)ovirt.org&gt; > > > > > > > Sent: Tuesday, August 27, 2013 11:40:27 AM > > > > > > > Subject: Re: [Engine-devel] Opimizing Postgres Stored > > > > > > > Procedures > > > > > > > > > > > > > > Hi Eli, > > > > > > > > > > > > > > Most of the functions that we have in the DB are doing very > > > > > > > simple > > > > > > > jobs > > > > > > > like > > > > > > > run a query, insert/update and I see that now you have all > > > > > > > QUERY > > > > > > > functions > > > > > > > as STABLE. > > > > > > > My questions: > > > > > > > Is this required for new functions from now on? > > > > > > Yes and a email asking that was posted to engine_devel > > > > > > > > > > > > > Is this done in order to improve performance? > > > > > > Yes > > > > > > > > > > Do you have any documents/benchmarks on how and why does this > > > > > improve > > > > > performance? > > > > > STABLE functions should improve performance if they return the same > > > > > result > > > > > for the same parameters in the same statement. > > > > > E.g. if you have a stable function like "select foo(x) from y" then > > > > > the > > > > > function can be invoked only once to evaluate each distinct value > > > > > of > > > > > y.x > > > > > - > > > > > this is kind of useful > > > > > Functions running queries for the ovirt engine are typically > > > > > invoked > > > > > from > > > > > client side, therefore they are only ivoked once from the > > > > > parameters > > > > > list > > > > > and therefore will be only executed once for that single statement. > > > > > > > > > > > > > > > > > > > > > > > > > Thank you, > > > > > > > Laszlo > > > > > > > > > > > > > > ----- Original Message ----- > > > > > > > > From: "Eli Mesika" <emesika(a)redhat.com&gt; > > > > > > > > To: "engine-devel" <engine-devel(a)ovirt.org&gt; > > > > > > > > Sent: Monday, August 26, 2013 11:22:20 AM > > > > > > > > Subject: [Engine-devel] Opimizing Postgres Stored Procedures > > > > > > > > > > > > > > > > Hi > > > > > > > > > > > > > > > > I had merged the following patch > > > > > > > > http://gerrit.ovirt.org/#/c/17962/ > > > > > > > > > > > > > > > > This patch introduce usage of the IMMUTABLE, STABLE and > > > > > > > > STRICT > > > > > > > > keywords > > > > > > > > in > > > > > > > > order to boost performance of the Postgres SPs. > > > > > > > > > > > > > > > > Please make sure that your current/and future DB scripts > > > > > > > > applied > > > > > > > > that. > > > > > > > > > > > > > > > > > > > > > > > > Volatility > > > > > > > > ---------- > > > > > > > > * A function should be marked as IMMUTABLE if it doesn't > > > > > > > > change > > > > > > > > the > > > > > > > > database, > > > > > > > > and if it doesn't perform any lookups (even for database > > > > > > > > configuration > > > > > > > > values) during its operation. > > > > > > > > * A function should be marked STABLE if it doesn't change the > > > > > > > > database, > > > > > > > > but > > > > > > > > might perform lookups (IMMUTABLE is preferable if function > > > > > > > > meets > > > > > > > > the > > > > > > > > requirements). > > > > > > > > * A function doesn't need to be marked VOLATILE, because > > > > > > > > that's > > > > > > > > the > > > > > > > > default. > > > > > > > > > > > > > > > > STRICTNESS > > > > > > > > ---------- > > > > > > > > A function should be marked STRICT if it should return NULL > > > > > > > > when > > > > > > > > it > > > > > > > > is > > > > > > > > passed > > > > > > > > a NULL argument, and then the function won't even be called > > > > > > > > if > > > > > > > > it > > > > > > > > is > > > > > > > > indeed > > > > > > > > passed a NULL argument. > > > > > > > > > > > > > > > > > > > > > > > > I am available for any questions. > > > > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > > Eli > > > > > > > > _______________________________________________ > > > > > > > > Engine-devel mailing list > > > > > > > > Engine-devel(a)ovirt.org > > > > > > > > http://lists.ovirt.org/mailman/listinfo/engine-devel > > > > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > Engine-devel mailing list > > > > > Engine-devel(a)ovirt.org > > > > > http://lists.ovirt.org/mailman/listinfo/engine-devel > > > > > > > > > > > > > > >