Database upgrade problem for 2.4.8-2 to 3.1.0-1

bdm · March 29, 2018, 11:57am

In our 2.4.8-2 database (postgres), which we started populating at 2.3.6, the table review_assignments has many NULL values of review_round_id.

I notice there was bug 8247 at 2.4.3 regarding NULL values of this field, which was handled by allowing NULL values.

The problem is that the 2.4.8-2 to 3.1.0-1 database upgrade makes a new table review_round_files that has attribute NOTNULL for field review_round_id (as specified in lib/pkp/xml/schema/reviews.xml). It causes the database upgrade to grind to a halt when this table is being populated.

To add some colour, the table review_rounds (in the 2.4.8-2 database) has many values of review_round_id that are absent from review_assignments. (However, there are no NULL values of review_round_id in that table.)

How can I proceed? I don’t mind doing some database hacking, but I don’t know what these tables are supposed to look like.

Thanks, Brendan.

ajnyga · March 29, 2018, 2:37pm

Have you checked whether you actually need those entries? I mean if review_assignment does not have a review_round_id I am not sure if the system can use them at all?

bdm · March 29, 2018, 11:12pm

Looking further, the issue is more dramatic than I wrote.

Our 2.4.8-2 table review_assignments has 5963 rows of which 1241 have review_round_id = NULL and all the rest have review_round_id = 0. In contrast, the table review_rounds has 5236 rows which all have distinct positive (nonzero) values of review_round_id.

So it looks like the review_round_id value in table review_assignments was completely useless. We did not notice any wrong behaviour in 2.4.8-2 that could be due to this, but it is easy to miss something that we don’t expect.

I don’t know if it is relevant, but when we first started using OJS (2.3.6) we imported multiple previous issues using the XML import plugin. I assume those would have no review information. But NULL values of review_round_id appear also for many articles processed using 2.3.6.

Brendan.

ajnyga · March 30, 2018, 4:35am

Did you do a recent upgrade from 2.3.6. to 2.4.8? If so, do you have the old database available and is the error already there?

If in review_rounds table you have review_round_id, round and submission_id available
And in review_assignments you still have submission_id and round available
then you could maybe try setting the missing values by comparing with submission_id and round the two tables?

tagging @bozana she is the zen master of upgrades.

bdm · March 30, 2018, 7:28am

Yes, we did an upgrade 2.3.6. to 2.4.8-2 recently. I looked at database dumps from 2014 and 2016 (when we were still running 2.3.6) and there is no field called review_round_id in any table at all. Here are the schemas for those two tables:

CREATE TABLE review_assignments (
    review_id integer NOT NULL,
    submission_id bigint NOT NULL,
    reviewer_id bigint NOT NULL,
    competing_interests text,
    regret_message text,
    recommendation smallint,
    date_assigned timestamp without time zone,
    date_notified timestamp without time zone,
    date_confirmed timestamp without time zone,
    date_completed timestamp without time zone,
    date_acknowledged timestamp without time zone,
    date_due timestamp without time zone,
    date_response_due timestamp without time zone,
    last_modified timestamp without time zone,
    reminder_was_automatic smallint DEFAULT 0 NOT NULL,
    declined smallint DEFAULT 0 NOT NULL,
    replaced smallint DEFAULT 0 NOT NULL,
    cancelled smallint DEFAULT 0 NOT NULL,
    reviewer_file_id bigint,
    date_rated timestamp without time zone,
    date_reminded timestamp without time zone,
    quality smallint,
    review_type smallint DEFAULT 1 NOT NULL,
    review_method smallint DEFAULT 1 NOT NULL,
    round smallint DEFAULT 1 NOT NULL,
    step smallint DEFAULT 1 NOT NULL,
    review_form_id bigint
);

CREATE TABLE review_rounds (
    submission_id bigint NOT NULL,
    round smallint NOT NULL,
    review_revision bigint,
    review_type bigint,
    status bigint
);

The fields review_round_id must have been added during the upgrade to 2.4.8-2.

As you suggested, I fixed review_assignments as much as possible using review_rounds:

UPDATE review_assignments as a
SET review_round_id=r.review_round_id
FROM review_rounds as r
WHERE r.submission_id=a.submission_id and r.round=a.round;
DELETE FROM review_assignments WHERE review_round_id is null;

There were only 53 null values at the DELETE step, so it was pretty successful.
The upgrade process proclaimed success.

I’ll report on how well other things work after we do some testing.

Cheers, Brendan.

ajnyga · March 31, 2018, 5:19am

Yes, my experience with OJS starts with 2.4.5, so I do not know the details there. But clearly something has gone wrong when the upgrade to OJS3 requires a value there as you said.

Just remember to do a lot of backups! Hopefully Bozana will check this after Easter as well. She knows the history of the schema a lot better.

bdm · March 31, 2018, 1:06pm

After making the above changes to the review_assignments table and then upgrading, there are still problems. Before describing them, I should mention that when I tried the 3.1.0-1 code straight out of the package the upgrade gave "ERROR: type ‘serial’ does not exist”. I applied the patch given by Alec Smecher in this thread – only the first of the two chunks worked but that message went away.

My software is Debian 9.3, Apache 2.4.25, PHP 5.6.34 and Postgres 9.6.7. I’m using the default theme at the moment.

Now the problems I noted, some of which may be features. I log in as a Journal Editor and click Submissions. I see four links “My Queue”, “Unassigned”, “All Active” and “Archives”.

An article was submitted in 2013 under 2.3.6. The editor requested three reviews but rejected the paper before they came (or maybe they came to him directly by email). In 3.1.0-1 it appears under “Archives” with “Declined” in orange, but “A review is overdue” is written below it. The filter “Overdue” accepts it, but the filter “Incomplete” rejects it. The filter “Review” accepts it, but the other stage filters reject it. This information is contradictory: since the submission has been declined, it is not overdue and not at the review stage. At least one reviewer of this long-rejected submission got email from the Acron plugin complaining that his review was 4 years overdue.
Lots of submissions in the “Archives” list have “Waiting for reviewers to be assigned” written below them even if they have been declined. But editors often reject submissions without assigning reviewers, and that shouldn’t raise an alarm.
Selecting the Archives list and then the filter Overdue, a lot of Published submissions appear in addition to Declined submissions. How can Published submissions be overdue?
In the Review panel of most rejected or published submissions (I tried about 20) there are active Accept Submission and Reject Submission buttons. Is this intentional? It seems dangerous to provide such buttons for already-finalised submissions.
The All Active list begins with lots of partial submissions that are not brought up by the search engine in 2.4.8 at all. (Nor does the ArticleReport plugin list them.) Moreover, clicking on those search results takes one to some stage in the Submit an Article workflow, as if the editor is the author. I even found one which took me to Step 5, “Submission Complete” yet it wasn’t completed according to its 2.4.8 record. Is that intentional?

Thanks. Brendan.

bozana · March 31, 2018, 4:51pm

Hi @bdm

Hmmm…
Your original situation seems to be OK – in my local OJS 2.4.8.3 installation I have the similar situation: review_assignments table has review_round_id = 0 in all rows, and review_rounds table has less rows than review_assignments table.
I do not have any entries in my review_assignments table with review_round_id = NULL, but this is because I did not upgrade from OJS 2.3.x. I cannot tell, but maybe it is also OK that they are/were there. Did you double check if you need those review_assignments rows before deleting them?
The upgrade to OJS 3 should populate those entries in the review_assignments table appropriately, e.g. here ojs/3.0.0_update.xml at ojs-3_1_0-1 · pkp/ojs · GitHub (but there are also other places in the code).
Thus, actually the errors that you got during the upgrade to 3.1.0.1 are very important to see and understand the situation, and to eventually correct it.
Is it possible for you to start the upgrade anew, from your original 2.4.8.2 installation (without that try to fix the review_assignments table) and to tell me what are the upgrade errors?

Best,
Bozana

bdm · April 1, 2018, 2:53am

If I leave review_assignments alone altogether, the update process stops with a message that it is trying to insert null values into the review_round_id field of table review_round_files (which has NOT NULL attribute).

Just changing the null values in review_assignments to 0 is the first thing I tried, and it produced symptom 1 in my list including the spurious review reminder. I did it again now from the beginning. No error messages appeared on the terminal screen during this process, nor in the apache2 log (which is where php errors are directed).

Now review_assignments has no null values of review_round_id, but it has 47 rows (out of more than 7000) with review_round_id=0. For example I see the following (numbers changed for privacy reasons) for a submission that was accepted after 3 review rounds. In the first round 4 reviewers sent regrets and reviewer 4810 recommended major revisions. In the second round 4810 recommended minor revisions. In the third round, 4810 recommended acceptance. All these events occurred before we upgraded from 2.3.6 to 2.4.8-2.

 submission_id | round | review_round_id | stage_id | reviewer_id
---------------+-------+-----------------+----------+-------------
          5739 |     2 |            4587 |        3 |        4810
          5739 |     1 |               0 |        3 |        4787
          5739 |     1 |               0 |        3 |        4783
          5739 |     1 |               0 |        3 |        4788
          5739 |     1 |               0 |        3 |        4805
          5739 |     1 |               0 |        3 |        4810
          5739 |     3 |            4858 |        3 |        4810

Entries in review_round_files for this submission are these:

 submission_id | review_round_id | stage_id | file_id | revision
---------------+-----------------+----------+---------+----------
          5739 |            4587 |        3 |   19622 |        2
          5739 |            4858 |        3 |   19622 |        3
          5739 |            4587 |        3 |   21321 |        3
          5739 |            4858 |        3 |   21321 |        4
          5739 |               0 |        3 |   21049 |        1

In the 2.4.8 database, the same entries appear in review_assignments except that review_round_id is null in all 7 rows. Table review_rounds has rounds 2 and 3 with these values of review_round_id but doesn’t have round 1 at all.

I can provide more information as you need.

Thanks, Brendan.

bdm · April 1, 2018, 4:06am

I’m trying to figure out why an automatic reminder was sent to reviewers of a rejected article. I’m assuming this is the work of lib/pkp/classes/task/ReviewReminder.inc.php. However, I could be wasting my time since I only saw this wrong behaviour for the Acron plugin and didn’t try the cron job method.

I’m referring to the submission from my problem 1, not the one in the previous message.

The SQL string assembled in lib/pkp/classes/submission/reviewAssignment/ReviewAssignmentDAO.inc.php function getIncompleteReviewAssignments() does not pay any attention to whether the submission is rejected or published yet. Here is the query put together:

SELECT r.*, r2.review_revision, u.first_name, u.last_name
  FROM    review_assignments r
      LEFT JOIN users u ON (r.reviewer_id = u.user_id)
      LEFT JOIN review_rounds r2 ON r.review_round_id = r2.review_round_id
   WHERE r.date_notified IS NOT NULL AND
       r.date_completed IS NULL AND r.declined <> 1
   ORDER BY r.submission_id;

In the case of the submission mentioned in my problem 1, r.declined=0 so this submission is selected. However, for that submission submissions.status=STATUS_DECLINED, so it should be skipped by the test “if ($submission->getStatus() != STATUS_QUEUED) continue;” in line 147 of lib/pkp/classes/task/ReviewReminder.inc.php. So I don’t understand why a reminder would be sent.

However, as I said, it could be a bug in Acron instead if that is executing different code.

Cheers, Brendan.

bozana · April 4, 2018, 10:53am

Hi @bdm,

Thanks!
I will try to construct the same situation in my local installation as you described in your second last post here… to see how actually the DB entries should be…

Regarding your last post, the review reminder: You are right, that is the class executed. And as far as I understand you are correct, that that code line 147 should miss that submission that is declined. But let me double check this as well. I will try to reconstruct that: I need to have an overdue review for a submission that is declined in the meantime.

I will then come back…
Thanks!
Bozana

ajnyga · April 4, 2018, 11:00am

@bozana note this: Move submission status check outside if clause by ajnyga · Pull Request #3550 · pkp/pkp-lib · GitHub

bdm · April 4, 2018, 11:03am

Hi @bozana,

I believe ajnyga has solved the problem of the spurious reminders in this thread. Basically, line 147 is not always executed.

The other problems (or features!) I listed above remain.

Thanks, Brendan.

bozana · April 10, 2018, 10:27am

Hi @bdm

Hmmm… Somehow I cannot reproduce the problem with the review_round_id = 0 after the migration to OJS 3.1.1 In OJS 2.4.8 I created a similar case: two review rounds, where a reviewer declined the review, and another suggested “major revisions” in the first round and ‘minor revisions’ in the second round. I have review_round_id = 0 in OJS 2.4.8 DB, but when migrated to OJS 3.1.1 everything seems to be fine :-\
I cannot figure out why you have those review_round_id = 0 in your DB. Maybe because that round 1 is missing in your OJS 2.4.8 (if I understood it correctly)? EDIT: do you have those reviews/review round properly in OJS 2.3.6?

Hmmm…

Bozana

bdm · April 12, 2018, 12:54pm

@bozana

I am fairly confident that the sequence was like this:

When the article transactions occured under 2.3.6, round 1 of submission 5739 never made it into the table review_rounds, though there are five rows in review_assignments. In 2.3.6 there is no such field as review_round_id.
In upgrading 2.3.6->2.4.8, review_round_id was created, but round 1 of submission 5739 was not assigned an id, presumably because it was missing from review_rounds. The field review_round_id was also added to review_assignments but its values were not set to meaningful values (some are 0 and some are null).
In upgrading 2.4.8->3.1.0-1, the values of review_rounds.review_round_id were used to populate the field review_assignments.review_round_id. However, since round 1 of submission 5739 was missing from review_rounds, no new values were added to review_assignments.review_round_id for that round; they remained 0 or null. Null values cause a fatal error in the upgrade process.

Perhaps it is not worth pursuing this issue as I doubt much can be done about problems that originated back in 2.3.6. The exception would be if the value 0 appearing in review_assignments.review_round_id or review_round_files.review_round_id is going to cause trouble. It is a non-existent round id and will potentially cause errors. Should I remove such rows from the database? Or is it plausible to reconstruct them from other information?

I’m still concerned about the submission attributes displayed in the search results, but since they seem to have a different cause I’ll move them to a new thread.

Thanks, Brendan.

bozana · April 16, 2018, 11:19am

Hi @bdm

Yes, I think you are right!
If possible it would be good to repair those entries in the OJS 2.4.8 DB. Is it possible for you to test this SQL on OJS 2.4.8 DB:
INSERT INTO review_rounds (submission_id, round, review_revision) SELECT DISTINCT ra.submission_id, ra.round, af.revision FROM review_assignments ra, article_files af, articles a WHERE (ra.submission_id, ra.round) NOT IN (SELECT rr.submission_id, rr.round FROM review_rounds rr) AND a.article_id = ra.submission_id AND af.article_id = a.article_id AND af.file_id = a.review_file_id AND af.file_stage = 2 AND af.round = ra.round
and then try the upgrade to 3.1.1 again?

To have an overview what should be inserted or how many rows, maybe you could first execute only the select statement from above i.e.:
SELECT DISTINCT ra.submission_id, ra.round, af.revision FROM review_assignments ra, article_files af, articles a WHERE (ra.submission_id, ra.round) NOT IN (SELECT rr.submission_id, rr.round FROM review_rounds rr) AND a.article_id = ra.submission_id AND af.article_id = a.article_id AND af.file_id = a.review_file_id AND af.file_stage = 2 AND af.round = ra.round

Best,
Bozana

bdm · April 16, 2018, 12:19pm

@bozana The SELECT only finds one item: submission_id=5739, round=1, revision=1. (More information on that submission is in my April 1 posting.)

There are altogether 47 rows for 12 distinct submission_ids in review_assignments for which review_round_id is not set to a useful value during the upgrade. Submission 5739 is just one of the them.

Here is the output of
SELECT DISTINCT submission_id, round, review_round_id FROM review_assignments WHERE submission_id in (SELECT DISTINCT submission_id FROM review_assignments WHERE review_round_id = 0) ORDER BY submission_id, round;
run after upgrade (with null values of review_round_id changed to 0 to avoid the fatal error):

 submission_id | round | review_round_id
---------------+-------+-----------------
          1093 |     1 |               0
          1497 |     1 |               0
          1497 |     2 |            1229
          1498 |     1 |               0
          1499 |     1 |               0
          1505 |     1 |               0
          1506 |     1 |               0
          1508 |     1 |               0
          1509 |     1 |               0
          1513 |     1 |               0
          1514 |     1 |               0
          1514 |     2 |            1625
          1713 |     1 |               0
          5739 |     1 |               0
          5739 |     2 |            4587
          5739 |     3 |            4858

And here is
SELECT DISTINCT submission_id, review_round_id FROM review_round_files WHERE submission_id in (SELECT DISTINCT submission_id FROM review_assignments WHERE review_round_id = 0) ORDER BY submission_id;

 submission_id | review_round_id
---------------+-----------------
          1497 |               0
          1497 |            1229
          1498 |               0
          1499 |               0
          1505 |               0
          1506 |               0
          1508 |               0
          1509 |               0
          1513 |               0
          1514 |               0
          1514 |            1625
          1713 |               0
          5739 |            4587
          5739 |            4858
          5739 |               0

You can see there are not so many cases (out of 7000+ altogether). I can do some fixing by hand if I know what to copy from where to where.

Thanks, Brendan.

bozana · April 16, 2018, 3:15pm

Hi @bdm

Phew… It is difficult for me to get it all sorted in my head

So, in your last post, do you mean the upgrade to 2.4.8 or 3.1.1?

Could you please also tell me what you see when you execute this SQL on your 2.4.8 DB:
SELECT DISTINCT ra.submission_id, ra.round FROM review_assignments ra WHERE (ra.submission_id, ra.round) NOT IN (SELECT rr.submission_id, rr.round FROM review_rounds rr)

Thanks!
Bozana

bdm · April 17, 2018, 6:01am

@bozana

Yes, I’m confusing myself too, sorry. In all cases I meant the 2.4.8->3.1.1 upgrade.
The SQL output is from:

“The SELECT finds only…” is the 2.4.8 db.
The two displays are for the 3.1.1 db. (review_assignments.review_round_id is always 0 or null in the 2.4.8 db). So what you see here is all the rows in the 3.1.1 db for which review_round_id was not set to a positive value during the 2.4.8->3.1.1 upgrade.)

Now I will run
SELECT DISTINCT ra.submission_id, ra.round FROM review_assignments ra WHERE (ra.submission_id, ra.round) NOT IN (SELECT rr.submission_id, rr.round FROM review_rounds rr) order by submission_id;
on the 2.4.8 db:

 submission_id | round
---------------+-------
          1093 |     1
          1497 |     1
          1498 |     1
          1499 |     1
          1505 |     1
          1506 |     1
          1508 |     1
          1509 |     1
          1513 |     1
          1514 |     1
          1713 |     1
          5739 |     1

Comparing this to the output from my previous post, it seems to have correctly identified all the problematic rounds.

Thanks, Brendan.

bozana · April 19, 2018, 9:00am

Hi @bdm

Hmmm… But, then it seems that those articles do not have anything in the DB column review_file_id in the 2.4.8 DB table articles, correct?

Thanks!
Bozana