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

[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

[pariv_0352]

I updated to 25.0.9 as well, and still have the issue.

[atripp]

@pariv_0352 the code is not fixed in 25.0.9

However, inedo/proget:25.0.10-ci.5 will have the new code that should prevent this error

[pariv_0352]

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

the code is not fixed in 25.0.9

Yes, I got it, just to clarify that I still have the error, so you can use me as tester )

Should I try 25.0.10-ci.5+ or wait for release?

[stevedennis]

Hi @pariv_0352,

Thanks! I'd try out 25.0.10-ci.10 -- it should work in theory! And if not then we'll just need to fix it anyway :)

Thanks,
Steve

[pariv_0352]

@stevedennis
I’ll try it as soon as it’s out :)

[pariv_0352]

I have installed 25.0.10 release. I can't reproduce the error anymore.

[thomas_3037]

I upgraded to 25.0.11-ci12 and have the problem. Its even worst then before, the workaround doesn't work any time.

[atripp]

Hi @thomas_3037 ,

The specific issue was already fixed; I'm going to close it because whatever your experiencing is different and the "history" in this long winding thread will only make it harder to understand ,

Please "start from the scratch" as if you never read this.

Thanks,
Alana