nifi-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mark Payne (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (NIFI-322) New Database Connection Pooling Controller Service
Date Wed, 04 Mar 2015 21:28:38 GMT

    [ https://issues.apache.org/jira/browse/NIFI-322?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14347593#comment-14347593
] 

Mark Payne commented on NIFI-322:
---------------------------------

Toivo,

I reviewed the changes. I found a couple of typos, and a couple of other very minor suggestions:

- Your MAX_WAIT_MILLIS is set to sensitive. I'm guessing you copied & pasted the property
descriptor for the password and forgot to change that setting. But I would actually change
the Property a bit. Rather than asking for the number of milliseconds, you should ask for
the amount of time:

    public static final PropertyDescriptor MAX_WAIT_TIME = new PropertyDescriptor.Builder()
    .name("Max Wait Time")
    .description("The maximum amount of time that the pool will wait (when there are no available
connections) " 
         " for a connection to be returned before failing, or -1 to wait indefinitely. ")
    .defaultValue("500 millis")
    .required(true)
    .addValidator(StandardValidators.TIME_PERIOD_VALIDATOR)
    .sensitive(false)
    .build();

This allows the user to enter the time period in whatever units they want. You'd then use
it as:

long maxWaitMillis = context.getProperty(MAX_WAIT_TIME).asTimePeriod(TimeUnit.MILLISECONDS);

- You changed the class name, but you forgot to update the toString(). It's still using the
old class name.

- It looks like the configContext member variable is assigned but never actually used.

- Not sure about the class loading yet? I thought you wanted to use a custom class loader
so that the user can specify their own class file?

Otherwise, I think it's in good shape. Just a couple of very minor tweaks. Great job!

Thanks
-Mark


> New Database Connection Pooling Controller Service
> --------------------------------------------------
>
>                 Key: NIFI-322
>                 URL: https://issues.apache.org/jira/browse/NIFI-322
>             Project: Apache NiFi
>          Issue Type: New Feature
>          Components: Extensions
>            Reporter: Toivo Adams
>            Assignee: Mark Payne
>            Priority: Minor
>              Labels: controller
>         Attachments: DBCPServiceApacheDBCP14.java, DBCPServiceTest.java, DatabaseSystemDescriptor.java,
DatabaseSystems.java, NIFI-322_01mar2015.patch, NIFI-322_03mar2015.patch, TestDatabaseSystems.java,
TestProcessor.java
>
>
> Often DataFlows contain many processors which deal with database - select, update or
delete different data in different tables. 
> Yet database is same and connection pooling helps to speed up connecting to database
(open connection is fairly expensive). Also configuration must be done only in one place.

> Database Connection Pooling Controller Service helps to solve this in consistent way.

> related
> https://issues.apache.org/jira/browse/NIFI-293 : Add a JDBC Processor for executing arbitrary
SQL queries. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message