swisscom/open-service-broker

Service detail value failed to save if length > 255 chars

MatthiasWinzeler opened this issue · 2 comments

Hi guys

Is there a reason why Service detail value is set to VARCHAR(255)?
Other values (i.e. plan metadata) are set to TEXT.

In our UAA service provider, we save the service instance params as Service details. This can easily grow over 255 chars, which then causes saving to fail:

2018-05-28 14:51:08.587  WARN 6470 --- [-8080-exec-4854] o.h.engine.jdbc.spi.SqlExceptionHelper   : SQL Error: 1406, SQLState: 22001
2018-05-28 14:51:08.587 ERROR 6470 --- [-8080-exec-4854] o.h.engine.jdbc.spi.SqlExceptionHelper   : Data truncation: Data too long for column '_value' at row 1
2018-05-28 14:51:08.605 ERROR 6470 --- [-8080-exec-4854] c.s.c.s.b.controller.BaseController      : Uncaught error

org.springframework.dao.DataIntegrityViolationException: could not execute statement; SQL [n/a]; nested exception is org.hibernate.exception.DataException: could not execute statement
        at org.springframework.orm.jpa.vendor.HibernateJpaDialect.convertHibernateAccessException(HibernateJpaDialect.java:282) ~[spring-orm-4.3.4.RELEASE.jar!/:4.3.4.RELEASE]
        at org.springframework.orm.jpa.vendor.HibernateJpaDialect.translateExceptionIfPossible(HibernateJpaDialect.java:244) ~[spring-orm-4.3.4.RELEASE.jar!/:4.3.4.RELEASE]
        at org.springframework.orm.jpa.AbstractEntityManagerFactoryBean.translateExceptionIfPossible(AbstractEntityManagerFactoryBean.java:491) ~[spring-orm-4.3.4.RELEASE.jar!/:4.3.4.RELEASE]
        at org.springframework.dao.support.ChainedPersistenceExceptionTranslator.translateExceptionIfPossible(ChainedPersistenceExceptionTranslator.java:59) ~[spring-tx-4.3.4.RELEASE.jar!/:4.3.4.RELEASE]
        at org.springframework.dao.support.DataAccessUtils.translateIfNecessary(DataAccessUtils.java:213) ~[spring-tx-4.3.4.RELEASE.jar!/:4.3.4.RELEASE]
        at org.springframework.dao.support.PersistenceExceptionTranslationInterceptor.invoke(PersistenceExceptionTranslationInterceptor.java:147) ~[spring-tx-4.3.4.RELEASE.jar!/:4.3.4.RELEASE]
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179) ~[spring-aop-4.3.4.RELEASE.jar!/:4.3.4.RELEASE]
        at org.springframework.data.jpa.repository.support.CrudMethodMetadataPostProcessor$CrudMethodMetadataPopulatingMethodInterceptor.invoke(CrudMethodMetadataPostProcessor.java:133) ~[spring-data-jpa-1.10.5.RELEASE.jar!/:na]
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179) ~[spring-aop-4.3.4.RELEASE.jar!/:4.3.4.RELEASE]
        at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:92) ~[spring-aop-4.3.4.RELEASE.jar!/:4.3.4.RELEASE]
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179) ~[spring-aop-4.3.4.RELEASE.jar!/:4.3.4.RELEASE]
        at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:213) ~[spring-aop-4.3.4.RELEASE.jar!/:4.3.4.RELEASE]
        at com.sun.proxy.$Proxy132.save(Unknown Source) ~[na:na]
        at com.swisscom.cloud.sb.broker.provisioning.ProvisioningPersistenceService$_updateServiceDetails_closure1.doCall(ProvisioningPersistenceService.groovy:113) ~[broker-2.12.1.jar!/:na]

@mcelep do you maybe remember why we put this in place? Can we safely change it?

I'd assume the reasoning being performance ... text is 'slightly' slower than varchar ... also a text field can never be indexed/unique. It's usually used to store huge blobs like json, texts etc.

Reading up on the matter, it seems quite a tricky topic (https://mariadb.com/kb/en/library/varchar/) using a big varchar may lead to a huge storage and memory usage.
Using a text may have a negative effect on overall performance, as we are using service details all over.

thanks @seilc1 UNIQUENESS is not a reason.
I guess there is no other way to fix it than to extend the column and live with a potential performance impact, right?

If we outsource the keys to credhub it is solved, but we still should fix it for the non-Credhub case...