get_lsn_by_timestamp: clamp commit_lsn to be >= min_lsn

[arpad-m]

There was an edge case where get_lsn_by_timestamp/find_lsn_for_timestamp could have returned an lsn that is before the limits we enforce: when we did find SLRU entries with timestamps before the one we search for.

The API contract of get_lsn_by_timestamp is to not return something before the anchestor lsn.

cc https://neondb.slack.com/archives/C03F5SM1N02/p1713871064147029

[jcsp]

LGTM. Is there any way to craft a test case for this?

[github-actions]

2766 tests run: 2645 passed, 0 failed, 121 skipped (full report)

---

Flaky tests (1)

Postgres 16

• test_timeline_size_quota_on_startup: release

Code coverage* (full report)

• functions: 28.1% (6478 of 23035 functions)
• lines: 47.0% (46008 of 97944 lines)

* collected from Rust tests only

---

_{The comment gets automatically updated with the latest test results
623c3a6 at 2024-04-23T17:06:16.779Z :recycle:}

[arpad-m]

This is an edge case: found_smaller needs to be true (otherwise we already return min_lsn), so the search must have found a timestamp before the specified timestamp at the min_lsn. Also, the search may not yield any lsn after the lsn we are looking for.

I'll be thinking about a test case. Depending on urgency, @koivunej feel free to merge this if you think a fix is urgent.

[arpad-m]

Hmmm tried for two hours to create a reproducer but the issue is that this is really an edge case.

This (added to test_lsn_mapping) is still successful before this patch:

        # Regression test for an edge case fixed by #7488
        # Create a bunch of branches and probe the returned lsn at that timestamp
        found_with_present = False
        for i in range(10, len(tbl), 100):
            probe_timestamp_before = cast(datetime, tbl[i - 1][1])
            probe_timestamp = cast(datetime, tbl[i][1])
            result_before = client.timeline_get_lsn_by_timestamp(
                tenant_id, timeline_id, probe_timestamp_before
            )
            assert result_before["kind"] not in ["past", "nodata"]
            parent_lsn_before = Lsn(result_before["lsn"])

            result = client.timeline_get_lsn_by_timestamp(tenant_id, timeline_id, probe_timestamp)
            assert result["kind"] not in ["past", "nodata"]
            parent_lsn = Lsn(result["lsn"])

            for branch_lsn in range(int(parent_lsn_before), int(parent_lsn), 8):
                branch_lsn = Lsn(branch_lsn)
                timeline_id_child = env.neon_cli.create_branch(
                    f"test_lsn_mapping_child_loop_{i}_{branch_lsn}",
                    tenant_id=tenant_id,
                    ancestor_branch_name="test_lsn_mapping",
                    ancestor_start_lsn=branch_lsn,
                )

                result = client.timeline_get_lsn_by_timestamp(
                    tenant_id, timeline_id_child, probe_timestamp_before
                )
                assert result["kind"] != "nodata"
                if result["kind"] in ["present", "future"]:
                    found_with_present = True
                lsn = Lsn(result["lsn"])
                assert lsn >= branch_lsn, "Grandchild lsn needs to be valid"
                timeline_id_child = env.neon_cli.create_branch(
                    f"test_lsn_mapping_child_loop_{i}_{branch_lsn}_grandchild",
                    tenant_id=tenant_id,
                    ancestor_branch_name="test_lsn_mapping",
                    ancestor_start_lsn=lsn,
                )
        assert found_with_present