Opened 3 years ago

Closed 3 years ago

#13500 closed Developer Task (fixed)

Deleting channels can orphan references in recorded and oldrecorded

Reported by: gigem Owned by: gigem
Priority: minor Milestone: 31.0
Component: MythTV - General Version: v30-fixes
Severity: medium Keywords:
Cc: Ticket locked: no


When channels are deleted, it can leave orphaned references to the missing channels in the recorded and oldrecorded tables.

Attachments (2)

13500-deleted1.patch (56.6 KB) - added by gigem 3 years ago.
13500-deleted2.patch (57.2 KB) - added by gigem 3 years ago.

Download all attachments as: .zip

Change History (9)

Changed 3 years ago by gigem

Attachment: 13500-deleted1.patch added

comment:1 Changed 3 years ago by gigem

13500-deleted1.patch is my first attempt at fixing this problem. Rather than deleting channels immediately, they are now flagged for deletion during housekeeping at a later time. Channels without any references are deleted after 1 day. Channels with references are kept indefinitely. I have done basic testing with this patch and would now like others to review it.

comment:2 Changed 3 years ago by Klaas de Waal

I would appreciate a more extensive description of what the problem is that is being solved. As I understand it, the goal is to show the channel name of old recordings when the channel does not exist anymore. I do have a number of recordings like that and they then show up with either the wrong channel name or just a number, but they play correct.

About the patch, I have done some testing with mythtv-setup and it looks OK, it does not break anything for me.

There is something strange about the chanid values. On a very first scan (with a really empty list of channels) then channel 1 gets chanid 10001 which is videosource ID x 10000 plus the channel number, etc. After deleting all channels and scanning again, one channel gets chanid 10000 and the other channels get numbers in the 150000 region. This is peculiar.

It would be great that when I delete all channels and then scan again, that the channels just deleted and found again would be come "undeleted". So the state "deleted" would then only be used for channels that are really gone. The advantage would be that the data that has been entered by the user in the channel, such as the visible state, the xmltv id and the icon path, would be preserved. I think that a lot of users would be very happy with that.

comment:3 Changed 3 years ago by Klaas de Waal

As mentioned earlier scanning works OK but looking later at the logs I found error messages like this:

2019-11-06 21:59:23.660801 E  Error preparing query: SELECT MAX(chanid) FROM channel AND sourceid = :SOURCEID
2019-11-06 21:59:23.660804 E  Driver error was [2/1064]:
QMYSQL3: Unable to prepare statement
Database error was:
You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'AND sourceid = ?' at line 1

2019-11-06 21:59:23.660844 E  DB Error (Getting chanid for new channel (2)):
Query was:
SELECT MAX(chanid) FROM channel AND sourceid = 1
Bindings were:
Driver error was [2/1064]:
QMYSQL: Unable to execute query
Database error was:
You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'AND sourceid = 1' at line 1

The sequence to produce this:

  • first delete all channels of the videosource with mysql statement "delete from channel where sourceid=1;"
  • do a "Scan of all existing transports"
  • delete all channels with the Delete button in mythtv-setup page Channel Editor
  • do a "Scan of all existing transports"
  • do "Insert All" for the new channels found

Now the database errors come.

Changed 3 years ago by gigem

Attachment: 13500-deleted2.patch added

comment:4 Changed 3 years ago by gigem

Thanks for testing, Klaas. The new patch addresses the bug you spotted. It also includes the schema update in the bindings that I forget and is updated to current master.

The problem being solved is to preserve the channel information for recordings (recorded table) as you noted but also for the recording history (oldrecorded table). When that information is missing, MythTV reports it as either empty or as #<chanid>.

In addition, this implementation is greatly influenced by feature requests relayed to me by others. While I have no immediate plans to implement those features, I tried to not make their possible implementation more difficult. Those feature requests are the following.

Be able to "undelete" deleted channels. That's why unreferenced channels are kept for at least one day rather than deleting them immediately.

Be able to track the period of time that channels were active for both past changes and future changes that are known or expected. That's why channel.deleted is a timestamp instead of simply a boolean. channel.deleted indicates when that channels active time ended. A future change might add channel.created to indicate when the active time began/begins.

As for deleting, rescanning and undeleting channels, I feel that's an improvement that belongs in the scanner to consider. Ideally, deleting and undeleting shouldn't even be necessary. If it doesn't already, the scanner should do a better job of reconciling the changes found in a rescan.

Finally, you mentioned the chanid = sourceid * n + channum "feature". That was a bad, design decision when it was added and it's still a bad design decision. If this change is the one to finally kill it off, then hooray.

comment:5 Changed 3 years ago by Klaas de Waal

Have repeated the tests and it looks OK to me. No more sql errors and the chanid values are now as expected.

About undeleting channels when they are found again in a scan, this is indeed something that can and should be done in the channel scanner. I have contemplated for some time the idea of creating a separate table to preserve the non-scanned data of a channel such as the icon path. By getting the data from a deleted channel or by undeleting the channel this problem is nicely solved.

I think that mythtv-setup scanning in the current master is now quite good and most users will never need to delete channels. But accidents do happen and data that people have entered manually should never get lost. So keeping the deleted channels for a while looks to me a step in the right direction.

comment:6 in reply to:  5 Changed 3 years ago by Gary Buhrmaster

Replying to Klaas de Waal:

... So keeping the deleted channels for a while looks to me a step in the right direction.

While I have mentioned this previously, in that I prefer the elegance and completeness of effective dated logic(*), I would agree that this is an excellent step forward that achieves a number of the goals without requiring advanced DB skills in the project moving forward (the perfect is the enemy of the good enough, and this is more than just good enough for the (likely) 98%).

(*) I appreciate the more pure approach, and (sadly) can see mathematical predicate calculus (i.e. relational calculus) in my head when necessary (it is only math, just like git uses a distributed graph theory tree model).

comment:7 Changed 3 years ago by David Engel <dengel@…>

Resolution: fixed
Status: assignedclosed

In 2e26f7f12f/mythtv:

Preserve data for deleted channels that are still referenced.

Don't immediately remove database entries for channels when they are
deleted. Instead, mark them as deleted so they are no longer used.
Change housekeeping to remove the entries for delted channels at a
later time when they are no longer referenced.

Fixes #13500

Note: See TracTickets for help on using tickets.