APEXMALHAR-2066 JdbcPolling,idempotent,partitionable #282

devtagare · 2016-05-18T22:28:38Z

JdbcPolling,idempotent,partitionable #282

devtagare · 2016-05-18T22:41:51Z

JdbcPolling,idempotent,partitionable

gauravgopi123 · 2016-05-18T22:55:39Z

library/src/main/java/com/datatorrent/lib/db/jdbc/AbstractJdbcPollInputOperator.java

+  protected transient boolean isReplayed;
+  protected transient boolean isPollable;
+  protected int batchSize;
+  protected int fetchSize;


add javadocs for properties

gauravgopi123 · 2016-05-26T21:40:26Z

library/src/main/java/com/datatorrent/lib/db/jdbc/AbstractJdbcPollInputOperator.java

+
+    } catch (SQLException e) {
+      LOG.error("Exception in initializing the range query for a given partition", e);
+      throw new RuntimeException();


throw new RuntimeException(e);

No need to log error

Should also wrap original exception, use Throwables.propagate

sandeepdeshmukh · 2016-06-01T11:43:44Z

What are the challenges in extending AbstractJdbcInputOperator to support new features ? As AbstractJdbcInputOperator is Evolving, there is no backward compatibility issue.

pramodin · 2016-06-01T20:23:43Z

@sandeepdeshmukh I will let @devtagare comment but I believe this operator makes assumptions on how data is organized in the database and expects the implementations to provide that information whereas the original operator makes no such assumptions.

gauravgopi123 · 2016-06-06T20:57:55Z

Any updates?

devtagare · 2016-06-06T21:13:22Z

I have updated the JIRA with assumptions for the operator - https://issues.apache.org/jira/browse/APEXMALHAR-2066.

Waiting for comments from more folks before making final changes.

bhupeshchawda · 2016-06-07T23:15:47Z

library/src/main/java/com/datatorrent/lib/db/jdbc/AbstractJdbcPollInputOperator.java

+  protected String upper;
+  protected boolean recovered;
+  protected boolean isPolled;
+  protected String whereCondition = null;


Can you also clarify which subset of SQL will be supported here? How complex can the where clause be?

gauravgopi123 · 2016-06-08T18:50:26Z

@devtagare : I wen through the Jira but it still doesn't mention how is this different from existing version and why can't existing version be enhanced?

bhupeshchawda · 2016-06-09T00:01:11Z

library/src/main/java/com/datatorrent/lib/db/jdbc/JdbcPollInputOperator.java

+       * Dont flush if its pollable*/
+      if (!hasNext) {
+        if ((isPollable && isPolled) || !isPollable) {
+          emitQueue.add(metaList);


This might throw an exception if the queue is full. Use offer() instead?

bhupeshchawda · 2016-06-09T00:16:13Z

library/src/main/java/com/datatorrent/lib/db/jdbc/JdbcMetaDataUtility.java

+      ResultSet rs = preparedStatement.executeQuery();
+
+      while (rs.next()) {
+        rowCount = Long.parseLong(rs.getString("RowCount"));


Suggest to use index instead of a column alias. Also seems like this is a simple count utility. Can you rename it to something like "getTupleCount()" ?

bhupeshchawda · 2016-07-11T12:31:44Z

@devtagare Apart from the existing comments I am fine with the changes.

The only major concern I have is related to the way polling is done might be dependent on a lot of things like the target database implementation and the organization of data in the database.
Request you to please mention the design of the operator (the core part on how you are planning to achieve polling functionality) on dev@apex. I guess, this will be done sooner or later as part of documentation :-)
Apologies for the delay.

devtagare · 2016-07-12T00:14:45Z

@bhupeshchawda - review comments are incorporated.Could you please check.

bhupeshchawda · 2016-07-12T06:48:33Z

library/src/test/java/com/datatorrent/lib/db/jdbc/JdbcPollerTest.java

+    pollableInstance.beginWindow(1);
+    Thread.sleep(700);
+    pollableInstance.endWindow();
+    pollableInstance.emitTuples();


This is emitting outside the window. Needs to be between begin and end window.

bhupeshchawda · 2016-07-12T06:59:03Z

@devtagare I can still see a lot of open comments. Can you please address them?
Also, as I suggested, please provide the description of the design on the dev mailing list. I think this will serve the larger purpose of letting the community know about this operator and its use, in contrast to the existing Jdbc operator in Malhar.

devtagare · 2016-07-12T18:22:17Z

@bhupeshchawda - fixed the latest comments and pushed the changes.Mail sent to dev@.

tushargosavi · 2016-07-13T06:24:09Z

library/src/main/java/com/datatorrent/lib/db/jdbc/AbstractJdbcPollInputOperator.java

+          long endTs = System.currentTimeMillis();
+          long ioTime = endTs - startTs;
+          long sleepTime = pollInterval - ioTime;
+          LOG.debug("pollInterval = " + pollInterval + " I/O time = " + ioTime + " sleepTime = " + sleepTime);


use {} syntax for log message, the string gets evaluated even if log level is not enabled.

tweise · 2016-07-13T16:59:12Z

Why would this not be part of the javadoc?

sent from mobile
On Jul 11, 2016 2:31 PM, "Bhupesh" notifications@github.com wrote:

@devtagare https://github.com/devtagare Apart from the existing
comments I am fine with the changes.

The only major concern I have is related to the way polling is done might
be dependent on a lot of things like the target database implementation and
the organization of data in the database.
Request you to please mention the design of the operator (the core part on
how you are planning to achieve polling functionality) on dev@apex. I
guess, this will be done sooner or later as part of documentation :-)
Apologies for the delay.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#282 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAQGD57_10pm8NcpMusPBEAJTfA1IkU_ks5qUjexgaJpZM4Ihwkj
.

bhupeshchawda · 2016-07-13T17:16:44Z

Yes, this should be part of javadoc as well.

devtagare closed this May 18, 2016

devtagare reopened this May 18, 2016

gauravgopi123 reviewed May 18, 2016
View reviewed changes

devtagare force-pushed the APEXMALHAR-2066-JdbcPolling branch from 1538b57 to 8ad2fae Compare May 26, 2016 20:28

gauravgopi123 reviewed May 26, 2016
View reviewed changes

bhupeshchawda reviewed Jun 7, 2016
View reviewed changes

bhupeshchawda reviewed Jun 9, 2016
View reviewed changes

devtagare force-pushed the APEXMALHAR-2066-JdbcPolling branch 2 times, most recently from 4ed207d to 6687bad Compare July 11, 2016 23:44

bhupeshchawda reviewed Jul 12, 2016
View reviewed changes

devtagare force-pushed the APEXMALHAR-2066-JdbcPolling branch from 6687bad to 6d8f237 Compare July 12, 2016 18:19

tushargosavi reviewed Jul 13, 2016
View reviewed changes

APEXMALHAR-2066 JdbcPolling,idempotent,partitionable

4c7d268

devtagare force-pushed the APEXMALHAR-2066-JdbcPolling branch from 6d8f237 to 4c7d268 Compare July 14, 2016 18:43

asfgit merged commit 4c7d268 into apache:master Jul 15, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

APEXMALHAR-2066 JdbcPolling,idempotent,partitionable #282

APEXMALHAR-2066 JdbcPolling,idempotent,partitionable #282

devtagare commented May 18, 2016 •

edited

devtagare commented May 18, 2016

gauravgopi123 May 18, 2016

gauravgopi123 May 26, 2016

tweise May 29, 2016

sandeepdeshmukh commented Jun 1, 2016

pramodin commented Jun 1, 2016

gauravgopi123 commented Jun 6, 2016

devtagare commented Jun 6, 2016

bhupeshchawda Jun 7, 2016

gauravgopi123 commented Jun 8, 2016

bhupeshchawda Jun 9, 2016

bhupeshchawda Jun 9, 2016

bhupeshchawda commented Jul 11, 2016

devtagare commented Jul 12, 2016

bhupeshchawda Jul 12, 2016

bhupeshchawda commented Jul 12, 2016

devtagare commented Jul 12, 2016

tushargosavi Jul 13, 2016

tweise commented Jul 13, 2016

bhupeshchawda commented Jul 13, 2016

APEXMALHAR-2066 JdbcPolling,idempotent,partitionable #282

APEXMALHAR-2066 JdbcPolling,idempotent,partitionable #282

Conversation

devtagare commented May 18, 2016 • edited

devtagare commented May 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandeepdeshmukh commented Jun 1, 2016

pramodin commented Jun 1, 2016

gauravgopi123 commented Jun 6, 2016

devtagare commented Jun 6, 2016

Choose a reason for hiding this comment

gauravgopi123 commented Jun 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bhupeshchawda commented Jul 11, 2016

devtagare commented Jul 12, 2016

Choose a reason for hiding this comment

bhupeshchawda commented Jul 12, 2016

devtagare commented Jul 12, 2016

Choose a reason for hiding this comment

tweise commented Jul 13, 2016

bhupeshchawda commented Jul 13, 2016

devtagare commented May 18, 2016 •

edited