proget 500 Internal server error when pushing to a proget docker feed

[atripp]

Hmmm the only possibility I can see is that DockerBlobs_CreateOrUpdateBlob is returning NULL , which is failing conversion to int dockerBlobId. That's the only nullable converstion on that line.

There's gotta be some kind of bug with this postresql procedure. Maybe a race condition??

CREATE OR REPLACE FUNCTION "DockerBlobs_CreateOrUpdateBlob"
(
    "@Feed_Id" INT,
    "@Blob_Digest" VARCHAR(128),
    "@Blob_Size" BIGINT,
    "@MediaType_Name" VARCHAR(255) = NULL,
    "@Cached_Indicator" BOOLEAN = NULL,
    "@Download_Count" INT = NULL,
    "@DockerBlob_Id" INT = NULL
)
RETURNS INT
LANGUAGE plpgsql
AS $$
BEGIN

    SELECT "DockerBlob_Id"
      INTO "@DockerBlob_Id"
      FROM "DockerBlobs"
     WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL))
       AND "Blob_Digest" = "@Blob_Digest";

    WITH updated AS
    (
        UPDATE "DockerBlobs"
           SET "Blob_Size" = "@Blob_Size",
               "MediaType_Name" = COALESCE("@MediaType_Name", "MediaType_Name"),
               "Cached_Indicator" = COALESCE("@Cached_Indicator", "Cached_Indicator")
         WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL)) 
           AND "Blob_Digest" = "@Blob_Digest"
        RETURNING *
    )        
    INSERT INTO "DockerBlobs"
    (
        "Feed_Id",
        "Blob_Digest",
        "Download_Count",
        "Blob_Size",
        "MediaType_Name",
        "Cached_Indicator"
    )
    SELECT
        "@Feed_Id",
        "@Blob_Digest",
        COALESCE("@Download_Count", 0),
        "@Blob_Size",
        "@MediaType_Name",
        COALESCE("@Cached_Indicator", 'N')
    WHERE NOT EXISTS (SELECT * FROM updated)
    RETURNING "DockerBlob_Id" INTO "@DockerBlob_Id";

    RETURN "@DockerBlob_Id";

END $$;

Anyway we'll study that another day.. at least we think we know specifically where the issue is.

[felfert]

@atripp Tomorrow I will compare this function's code on my "production" proget with that on the sandbox. Just to be shure.

[atripp]

@inedo_1308 sounds good!

The code would almost certainly be the same, since it hasn't been updated since we did the PostgreSQL version of the script.

So, I think it's a race condition, though I don't know how it would happen. However, if it's a race condition, then it should be solved with an UPDLOCK (or whatever) in PostgreSQL.

1. SELECT finds no matching blob in the database (thus DockerBlob_Id is null)
2. ... small delay ...
3. UPDATE finds the matching blob because it was added (thus a row gets added to insert)
4. INSERT does run because there is a row in inserted
5. A NULL DockerBlob_Id is returned

If you're able to patch the procedure, could you add FOR UPDATE as follows? We are still relatively to PostgreSQL so I don't know if this the right way to do it in this case.

I think a second SELECT could also work, but I dunno.

CREATE OR REPLACE FUNCTION "DockerBlobs_CreateOrUpdateBlob"
(
    "@Feed_Id" INT,
    "@Blob_Digest" VARCHAR(128),
    "@Blob_Size" BIGINT,
    "@MediaType_Name" VARCHAR(255) = NULL,
    "@Cached_Indicator" BOOLEAN = NULL,
    "@Download_Count" INT = NULL,
    "@DockerBlob_Id" INT = NULL
)
RETURNS INT
LANGUAGE plpgsql
AS $$
BEGIN

    SELECT "DockerBlob_Id"
      INTO "@DockerBlob_Id"
      FROM "DockerBlobs"
     WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL))
       AND "Blob_Digest" = "@Blob_Digest"
   FOR UPDATE;

    WITH updated AS
    (
        UPDATE "DockerBlobs"
           SET "Blob_Size" = "@Blob_Size",
               "MediaType_Name" = COALESCE("@MediaType_Name", "MediaType_Name"),
               "Cached_Indicator" = COALESCE("@Cached_Indicator", "Cached_Indicator")
         WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL)) 
           AND "Blob_Digest" = "@Blob_Digest"
        RETURNING *
    )        
    INSERT INTO "DockerBlobs"
    (
        "Feed_Id",
        "Blob_Digest",
        "Download_Count",
        "Blob_Size",
        "MediaType_Name",
        "Cached_Indicator"
    )
    SELECT
        "@Feed_Id",
        "@Blob_Digest",
        COALESCE("@Download_Count", 0),
        "@Blob_Size",
        "@MediaType_Name",
        COALESCE("@Cached_Indicator", 'N')
    WHERE NOT EXISTS (SELECT * FROM updated)
    RETURNING "DockerBlob_Id" INTO "@DockerBlob_Id";

    RETURN "@DockerBlob_Id";

END $$;

[felfert]

@atripp said in proget 500 Internal server error when pushing to a proget docker feed:

The code would almost certainly be the same, since it hasn't been updated since we did the PostgreSQL version of the script.

You are right "production" and sandbox showed the same function

If you're able to patch the procedure, could you add FOR UPDATE as follows? We are still relatively to PostgreSQL so I don't know if this the right way to do it in this case.

Did that, verified that the function actually has changed and did another test. Unfortunately this did not help, error was exactly the same like in my above wireshark dump.
Or does one have to "compile" the function somehow after replacing? (I never dealt with SQL functions before and in general have very limited SQL knowledge.)

Cheers,
-Fritz

[felfert]

BTW:
Did you guys generate a different password for the postgres DB than for MSQL
during migration?

I'm asking, because i vaguely remember that i could access the postgres DB using the same password (taken from the mssql connection-string environment var).

Now, this didn't work anymore and I had to disable password-auth in pg_hba.conf in order to use pg_dump and psql inside the proget container.

Thanks
-Fritz

[atripp]

Hi @inedo_1308 ,

I forgot how it worked in the preview migration, but the connection string is stored in the database directory (/var/proget/database/.pgsqlconn).

Thanks,
Alana

[atripp]

Did that, verified that the function actually has changed and did another test. Unfortunately this did not help, error was exactly the same like in my above wireshark dump.
Or does one have to "compile" the function somehow after replacing? (I never dealt with SQL functions before and in general have very limited SQL knowledge.)

Ah that's a shame! We're kind of new to "patching" functions like this in postgresql, but I think that should have worked to change the code. And also the code change should have worked.

If you don't mind trying one other patch, where we select out the Blob_Id again in the end.

CREATE OR REPLACE FUNCTION "DockerBlobs_CreateOrUpdateBlob"
(
    "@Feed_Id" INT,
    "@Blob_Digest" VARCHAR(128),
    "@Blob_Size" BIGINT,
    "@MediaType_Name" VARCHAR(255) = NULL,
    "@Cached_Indicator" BOOLEAN = NULL,
    "@Download_Count" INT = NULL,
    "@DockerBlob_Id" INT = NULL
)
RETURNS INT
LANGUAGE plpgsql
AS $$
BEGIN

    SELECT "DockerBlob_Id"
      INTO "@DockerBlob_Id"
      FROM "DockerBlobs"
     WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL))
       AND "Blob_Digest" = "@Blob_Digest"
   FOR UPDATE;

    WITH updated AS
    (
        UPDATE "DockerBlobs"
           SET "Blob_Size" = "@Blob_Size",
               "MediaType_Name" = COALESCE("@MediaType_Name", "MediaType_Name"),
               "Cached_Indicator" = COALESCE("@Cached_Indicator", "Cached_Indicator")
         WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL)) 
           AND "Blob_Digest" = "@Blob_Digest"
        RETURNING *
    )        
    INSERT INTO "DockerBlobs"
    (
        "Feed_Id",
        "Blob_Digest",
        "Download_Count",
        "Blob_Size",
        "MediaType_Name",
        "Cached_Indicator"
    )
    SELECT
        "@Feed_Id",
        "@Blob_Digest",
        COALESCE("@Download_Count", 0),
        "@Blob_Size",
        "@MediaType_Name",
        COALESCE("@Cached_Indicator", 'N')
    WHERE NOT EXISTS (SELECT * FROM updated)
    RETURNING "DockerBlob_Id" INTO "@DockerBlob_Id";

    SELECT "DockerBlob_Id"
      INTO "@DockerBlob_Id"
      FROM "DockerBlobs"
     WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL))
       AND "Blob_Digest" = "@Blob_Digest"

    RETURN "@DockerBlob_Id";

END $$;

If this doesn't do the trick, I think we need to look a lot closer.

[thomas_3037]

I dump the schema from the Database. The Function looks nearly identical. Only the varchar sizes are different.

CREATE FUNCTION public."DockerBlobs_CreateOrUpdateBlob"("@Feed_Id" integer, "@Blob_Digest" character varying, "@Blob_Size" bigint, "@MediaType_Name" character varying DEFAULT NULL::character varying, "@Cached_Indicator" boolean DEFAULT 
NULL::boolean, "@Download_Count" integer DEFAULT NULL::integer, "@DockerBlob_Id" integer DEFAULT NULL::integer) RETURNS integer
    LANGUAGE plpgsql
    AS $$
BEGIN

    SELECT "DockerBlob_Id"
      INTO "@DockerBlob_Id"
      FROM "DockerBlobs"
     WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL))
       AND "Blob_Digest" = "@Blob_Digest";

    WITH updated AS
    (
        UPDATE "DockerBlobs"
           SET "Blob_Size" = "@Blob_Size",
               "MediaType_Name" = COALESCE("@MediaType_Name", "MediaType_Name"),
               "Cached_Indicator" = COALESCE("@Cached_Indicator", "Cached_Indicator")
         WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL)) 
           AND "Blob_Digest" = "@Blob_Digest"
        RETURNING *
    )        
    INSERT INTO "DockerBlobs"
    (
        "Feed_Id",
        "Blob_Digest",
        "Download_Count",
        "Blob_Size",
        "MediaType_Name",
        "Cached_Indicator"
    )
    SELECT
        "@Feed_Id",
        "@Blob_Digest",
        COALESCE("@Download_Count", 0),
        "@Blob_Size",
        "@MediaType_Name",
        COALESCE("@Cached_Indicator", 'N')
    WHERE NOT EXISTS (SELECT * FROM updated)
    RETURNING "DockerBlob_Id" INTO "@DockerBlob_Id";

    RETURN "@DockerBlob_Id";

END $$;

and the table schema

proget=# \d "DockerBlobs"
                                     Table "public.DockerBlobs"
       Column       |           Type           | Collation | Nullable |           Default            
--------------------+--------------------------+-----------+----------+------------------------------
 Feed_Id            | integer                  |           |          | 
 Blob_Digest        | character varying(128)   |           | not null | 
 Download_Count     | integer                  |           | not null | 
 LastRequested_Date | timestamp with time zone |           |          | 
 Blob_Size          | bigint                   |           | not null | 
 Cached_Indicator   | boolean                  |           | not null | 
 MediaType_Name     | character varying(255)   |           |          | 
 DockerBlob_Id      | integer                  |           | not null | generated always as identity
 LastScan_Date      | timestamp with time zone |           |          | 
Indexes:
    "PK__DockerBlobs" PRIMARY KEY, btree ("DockerBlob_Id")
    "IX__DockerBlobs__Blob_Digest" btree ("Blob_Digest")
    "UQ__DockerBlobs" UNIQUE CONSTRAINT, btree ("Feed_Id", "Blob_Digest")
Foreign-key constraints:
    "FK__DockerBlobs__Feeds" FOREIGN KEY ("Feed_Id") REFERENCES "Feeds"("Feed_Id") ON DELETE CASCADE
Referenced by:
    TABLE ""DockerImageLayers"" CONSTRAINT "FK__DockerBlobIndex__DockerBlobs" FOREIGN KEY ("DockerBlob_Id") REFERENCES "DockerBlobs"("DockerBlob_Id") ON DELETE CASCADE
    TABLE ""DockerBlobInfos"" CONSTRAINT "FK__DockerBlobInfos__DockerBlobs" FOREIGN KEY ("DockerBlob_Id") REFERENCES "DockerBlobs"("DockerBlob_Id") ON DELETE CASCADE
    TABLE ""DockerBlobPackages"" CONSTRAINT "FK__DockerBlobPackages__DockerBlobs" FOREIGN KEY ("DockerBlob_Id") REFERENCES "DockerBlobs"("DockerBlob_Id") ON DELETE CASCADE
    TABLE ""DockerImages"" CONSTRAINT "FK__DockerImages__DockerBlobs" FOREIGN KEY ("ContainerConfigBlob_Id") REFERENCES "DockerBlobs"("DockerBlob_Id") ON DELETE CASCADE

[felfert]

@atripp
If you don't mind trying one other patch, where we select out the Blob_Id again in the end.

Yippieh - That one did the Trick. Error is gone

There is a semicolon missing after the additional selct before the return

[felfert]

Going to push some more images to be on the safe side ...

[felfert]

@atripp Ok this is definitively working now. Pushed approx. 20 different images from 80MiB up to 2GiB. None of them produced an error anymore.

Excellent Support!!!

Thanks alot
-Fritz

[pariv_0352]

I was about to start a new thread, but this one might be related to my issue. I can't push a Docker image to ProGet (version 25.0.8). Docker keeps retrying, but without success. The Diagnostic Center shows no errors. In the logs, I see:

A 500 error occurred in docker_feed: Nullable object must have a value.

info: Microsoft.AspNetCore.Hosting.Diagnostics[2]

      Request finished HTTP/1.1 PUT http://mydomain.com/v2/docker_feed/my_service/blobs/uploads/1f3bb6af-6666-497e-bb8e-6b621d584b9c?digest=sha256%3Addfc8032fb97dfcf37359445fcb93b9742fcdf6f5bfdd288081557bf461edac6 - 500 1091 application/json 10.8169ms

Then I tried 25.0.9-ci.14, same result.

[felfert]

@pariv_0352 said in proget 500 Internal server error when pushing to a proget docker feed:

Then I tried 25.0.9-ci.14, same result.

I don't think the newer builds contain any fix for this yet. I was able to fix it myself locally in the postgres DB with guidance by @atripp, however she most likely did not see my success report yet, otherwise she very likely would have acknowledged it here.
The newer builds > 25.0.9-ci.7 are probably other developers working on different things.

And yes, the - 500 1091 part in your error message looks suspiciously like mine (before I fixed it locally).

So: Just wait, until @atripp confirms here, that she has actually applied that fix.

Cheers
-Fritz

[atripp]

@felfert thanks for confirming!!

FYI tThe fix has not been applied yet to code yet, but you can patch the stored procedure (painfully) as a workaround for now. We will try to find a better solution. The only thing I can imagine happening is that the PUT is happening immediately after the PATCH finishes, but before the client receives a 200 response. I have no idea though.

We'll figure something out, now that we know where it is thanks to your help!!

[felfert]

@atripp said in proget 500 Internal server error when pushing to a proget docker feed:

The only thing I can imagine happening is that the PUT is happening immediately after the PATCH finishes, but before the client receives a 200 response.

Just to confirm your assumption: Thats exactly what is happening as one can see in my first wireshark stream analysis of this above.

you can patch the stored procedure (painfully) as a workaround for now.

I'm writing a small shell (bash) script, that can be invoked inside the proget container in order to easily apply the fix. I will post this here later. Stay tuned.

Cheers
-Fritz

[felfert]

As announced above, I have created a little shell script for applying the fix for the bug discussed in this thread.

Prerequisites:

• ProGet >= 5.0.8 running in a docker container and already migrated to postgres DB
• backup-directory mapped to /var/proget/backups in the proget container

Usage:

• Copy the script from below and save it in the backup-directory using a name that makes sense for you (e.g. fix-docker-push.sh)
• Backup your database
• On the docker host running the proget container, execute the following cmdline (obviously inserting your container's name here and supplying the correct script-filename)

docker exec -it InsertYourContainersNameHere /bin/bash /var/proget/backups/fix-docker-push.sh

• The script will abort, if it cannot find the connection string of the postgres DB
• The script will abort, if it detects, that the fix has already applied
• Otherwise it will show a final warning and will abort, if you do not enter YES (all capitals)
• If you enter YES, it will apply the fix and psql will print CREATE FUNCTION

Here is the script:

#! /bin/bash

sqlconn=/var/proget/database/.pgsqlconn
if [ -f "${sqlconn}" ] ; then
    . ${sqlconn}
    PGPASSWORD="${Password}" psql -h ${Host} -p ${Port} -U ${Username} ${Database} <<-"EOF" |
		\sf "DockerBlobs_CreateOrUpdateBlob"
EOF
	grep -q "FOR UPDATE;"
    if [ $? = 0 ] ; then
        echo "It looks like the fix was already applied. Aborting"
        exit 0
    fi
    cat<<-EOF
        This script applies a fix for pushing images to a proget docker feed.

        WARNING: This modifies your proget postgres database. BACKUP YOUR DATABASE FIRST.

        Enter YES if you want to continue or hit Ctrl-C to abort.
EOF
    read resp; if [ "${resp}" != "YES" ] ; then
        echo "Aborting"
        exit 0
    fi
    PGPASSWORD="${Password}" psql -h ${Host} -p ${Port} -U ${Username} ${Database} <<-"EOF"
	    CREATE OR REPLACE FUNCTION "DockerBlobs_CreateOrUpdateBlob"
	    (
	        "@Feed_Id" INT,
	        "@Blob_Digest" VARCHAR(128),
	        "@Blob_Size" BIGINT,
	        "@MediaType_Name" VARCHAR(255) = NULL,
	        "@Cached_Indicator" BOOLEAN = NULL,
	        "@Download_Count" INT = NULL,
	        "@DockerBlob_Id" INT = NULL
	    )
	    RETURNS INT
	    LANGUAGE plpgsql
	    AS $$
	    BEGIN
	    
	        SELECT "DockerBlob_Id"
	        INTO "@DockerBlob_Id"
	        FROM "DockerBlobs"
	        WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL))
	        AND "Blob_Digest" = "@Blob_Digest"
	    FOR UPDATE;
	    
	        WITH updated AS
	        (
	            UPDATE "DockerBlobs"
	            SET "Blob_Size" = "@Blob_Size",
	                "MediaType_Name" = COALESCE("@MediaType_Name", "MediaType_Name"),
	                "Cached_Indicator" = COALESCE("@Cached_Indicator", "Cached_Indicator")
	            WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL)) 
	            AND "Blob_Digest" = "@Blob_Digest"
	            RETURNING *
	        )        
	        INSERT INTO "DockerBlobs"
	        (
	            "Feed_Id",
	            "Blob_Digest",
	            "Download_Count",
	            "Blob_Size",
	            "MediaType_Name",
	            "Cached_Indicator"
	        )
	        SELECT
	            "@Feed_Id",
	            "@Blob_Digest",
	            COALESCE("@Download_Count", 0),
	            "@Blob_Size",
	            "@MediaType_Name",
	            COALESCE("@Cached_Indicator", 'N')
	        WHERE NOT EXISTS (SELECT * FROM updated)
	        RETURNING "DockerBlob_Id" INTO "@DockerBlob_Id";
	    
	        SELECT "DockerBlob_Id"
	        INTO "@DockerBlob_Id"
	        FROM "DockerBlobs"
	        WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL))
	        AND "Blob_Digest" = "@Blob_Digest";
	    
	        RETURN "@DockerBlob_Id";
	    
	    END $$;
EOF
else
    echo "Postgres connection parameters (${sqlconn}) not found. Aborting"
fi

Disclaimer
I am NOT an inedo engineer and not affiliated with them.
You are responsible to backup you database before running this script.
You run this script at your own risk.
I take no responsibility whatsoever for any damages that might occur.

Have fun
-Fritz

[atripp]

@felfert amazing!! That script will come in handy when we need to help users patch their instance; we can also try to add something that allows you to patch via the UI as well!!

[felfert]

@atripp said in proget 500 Internal server error when pushing to a proget docker feed:

we can also try to add something that allows you to patch via the UI as well!!

Huh? Well, I don't think this would be such a good idea. I assume you guys already have some code in proget for upgrading your DB-Schema when a new version is required. That's where the function should be replaced. This script is only meant as a intermediate solution until you guys have updated the function.

Cheers
-Fritz

[atripp]

Hi @felfert ,

As an update, we'd are planning on this pattern instead of row/table locking (PG-3104). It gives us a lot more control and makes it a lot easier to avoid deadlocks.

I still can't reproduce the issue, but I see no reason this won't work.

CREATE OR REPLACE FUNCTION "DockerBlobs_CreateOrUpdateBlob"
(
    "@Feed_Id" INT,
    "@Blob_Digest" VARCHAR(128),
    "@Blob_Size" BIGINT,
    "@MediaType_Name" VARCHAR(255) = NULL,
    "@Cached_Indicator" BOOLEAN = NULL,
    "@Download_Count" INT = NULL,
    "@DockerBlob_Id" INT = NULL
)
RETURNS INT
LANGUAGE plpgsql
AS $$
BEGIN

	-- avoid race condition when two procs call at exact same time
	PERFORM PG_ADVISORY_XACT_LOCK(HASHTEXT(CONCAT_WS('DockerBlobs_CreateOrUpdateBlob', "@Feed_Id", LOWER("@Blob_Digest"))));

    SELECT "DockerBlob_Id"
      INTO "@DockerBlob_Id"
      FROM "DockerBlobs"
     WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL))
       AND "Blob_Digest" = "@Blob_Digest";

    WITH updated AS
    (
        UPDATE "DockerBlobs"
           SET "Blob_Size" = "@Blob_Size",
               "MediaType_Name" = COALESCE("@MediaType_Name", "MediaType_Name"),
               "Cached_Indicator" = COALESCE("@Cached_Indicator", "Cached_Indicator")
         WHERE ("Feed_Id" = "@Feed_Id" OR ("Feed_Id" IS NULL AND "@Feed_Id" IS NULL)) 
           AND "Blob_Digest" = "@Blob_Digest"
        RETURNING *
    )        
    INSERT INTO "DockerBlobs"
    (
        "Feed_Id",
        "Blob_Digest",
        "Download_Count",
        "Blob_Size",
        "MediaType_Name",
        "Cached_Indicator"
    )
    SELECT
        "@Feed_Id",
        "@Blob_Digest",
        COALESCE("@Download_Count", 0),
        "@Blob_Size",
        "@MediaType_Name",
        COALESCE("@Cached_Indicator", 'N')
    WHERE NOT EXISTS (SELECT * FROM updated)
    RETURNING "DockerBlob_Id" INTO "@DockerBlob_Id";

    RETURN "@DockerBlob_Id";

END $$;

[felfert]

@atripp I tested this one and it works.

However:
In the meantime, i have updated to the released 25.0.9 (aka latest) and now I cannot reproduce the error either - even if I explicitely revert the function to the old one where it produced the error before

So I'm afraid that i might not be a reliable tester for this specific error anymore.

Cheers
-Fritz