pgsql: Change layer name column data type #382
Conversation
|
If this change is acceptable, I'm happy to write the migration code as well. |
|
Hey, We do actually use a combination of a few things as well in production. I'd be happy to merge this thing, but wondering if there is any performance hit involved here at scale? Also, I believe you'd need to make a new migration, do not modify old ones, it might break existing deployments. We have a few casts here and there, I'd verify if there is any applying to that field. Thanks! |
|
@Quentin-M : Thanks for the review; I've updated the commit to do a migration rather than modify the initial schema. I looked into the Postgres docs and they claim there is no performance penalty: |
|
@Quentin-M Mind taking another look at this? |
|
This change looks good from the database-side; however, I do think there should be some limit via the API to prevent DoS. I think we need to discuss a fair length and then add another commit to enforce this at the API before merging. |
|
@jzelinskie That's a valid concern. How would you like to proceed? For my purposes, the column need not be unbounded. Perhaps just bumping the width up to |
|
@caipre This sounds fair. If you're quick, I'll cut this in v2.0.0. |
|
Looks like I missed the release, but I've updated the commit. |
Presently the layer and namespace tables use type `varchar(128)` for their respective name columns. For layer, this width works fine enough using the sha256 digests provided by docker. However, if one wishes to encode the image name into the layer name (eg, to avoid collisions like in [0]), the limit of 128 bytes starts to feel a bit cramped. Bump to 256 bytes, since that "ought to be enough for anybody." (TM) [0]: #319
pgsql: Change layer name column data type
pgsql: Change layer name column data type
Presently the
layer.namecolumn has typevarchar(128). This worksfine enough when using the sha256 layer digests provided by docker.
However, if one wishes to encode the image name into the layer (eg, to
avoid collisions like in #319),
the limit of 128 bytes becomes a bit cramped.
I've configured my client to name layers using the docker layer digest
concatenated with sha256(image.name, image.tag, layer.parent.digest):
This change allows the name column to support such a format.