> On June 16, 2015, 6:46 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/view/persistence/DataStoreImpl.java, > > line 618 > > <https://reviews.apache.org/r/35528/diff/1/?file=985857#file985857line618> > > > > It seems like this would truncate the value, thus loosing data. Is this > > OK? If so, we can close this issue, but I just wanted to verify that this > > behavior was expected. > > Tom Beerbower wrote: > Thanks for reviewing! > > The DataStore was meant to be a light weight persistence mechanism for > views so I think that having reasonable restrictions is okay. But you are > correct in that the data would be trunctated and the user might never know, > which could be bad. I guess the options are 1) truncate the String and log > it. 2) Fail fast if the String is too long. 3) Map the String fields to a > CLOB type, if possible. What do you think? > > Jonathan Hurley wrote: > Yes, I'd say use a CLOB if this could go beyond 4000. How does this work > for different DBs? Do we just rely on the mappings? If we used a CLOB, the > persisence layer will map this to things like LARGETEXT, TEXT, etc, which is > good. > > Unless you really don't think you'd need more than 4000. In which case > I'm fine with the truncation.
We don't really know because the user view could stick any length String into the entity field. I've update the diff to use CLOB. Yeah, the persistence layer should handle the correct mapping. I just tested on Postgres and it mapped to a TEXT type. Thanks guys! - Tom ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35528/#review88107 ----------------------------------------------------------- On June 16, 2015, 8:21 p.m., Tom Beerbower wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35528/ > ----------------------------------------------------------- > > (Updated June 16, 2015, 8:21 p.m.) > > > Review request for Ambari, Jonathan Hurley and Nate Cole. > > > Bugs: AMBARI-11957 > https://issues.apache.org/jira/browse/AMBARI-11957 > > > Repository: ambari > > > Description > ------- > > In one case we are seeing this exception while trying to store data from a > view ... > > java.lang.IllegalStateException: > Exception Description: No transaction is currently active > at > org.eclipse.persistence.internal.jpa.transaction.EntityTransactionImpl.rollback(EntityTransactionImpl.java:176) > at > org.apache.ambari.server.view.persistence.DataStoreImpl.store(DataStoreImpl.java:135) > at > org.apache.ambari.view.hive.persistence.DataStoreStorage.store(DataStoreStorage.java:76) > at > org.apache.ambari.view.hive.resources.CRUDResourceManager.save(CRUDResourceManager.java:117) > at > > The underlying exception ... > > Exception [EclipseLink-4002] (Eclipse Persistence Services - > 2.5.2.v20140319-9ad6abd): org.eclipse.persistence.exceptions.DatabaseException > Internal Exception: org.postgresql.util.PSQLException: ERROR: value too > long for type character varying(255) > Error Code: 0 > Call: UPDATE DS_JOBIMPL_4 SET DS_applicationId = ?, DS_duration = ?, > DS_sqlState = ?, DS_status = ?, DS_statusMessage = ? WHERE (DS_id = ?) > bind => [6 parameters bound] > > > This occurs because the view tries to save an entity with a String type > attribute which is mapped to a VARCHAR(255). The dynamic entity code maps > the String to a VARCHAR using the database default length. > > Also, once the exception is thrown, the DataStore catches it and tries to > rollback the transaction. It looks like the tx is already inactive at this > point so a different exception is thrown out. The original exception should > at least be logged by the DataStoreImpl. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/view/persistence/DataStoreImpl.java > 8353d63 > > ambari-server/src/test/java/org/apache/ambari/server/view/persistence/DataStoreImplTest.java > 2b19b0a > > Diff: https://reviews.apache.org/r/35528/diff/ > > > Testing > ------- > > Manual tested. > > New unit tests added. > > mvn clean test > > all pass > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 46:25 min > [INFO] Finished at: 2015-06-16T13:42:50-04:00 > [INFO] Final Memory: 54M/1453M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Tom Beerbower > >
