add atomic reference tracker #2893

gman0 · 2022-02-21T17:00:51Z

Describe what this PR does

RT, reference tracker, is key-based implementation of a reference counter.
Unlike an integer-based counter, RT counts references by tracking unique
keys. This allows accounting in situations where idempotency must be
preserved. It guarantees there will be no duplicit increments or decrements
of the counter.

It is directly needed by the snapshot-backed shallow volumes for CephFS, but is general enough for any other use-cases that need accounting.

Includes unit tests. They use "fake RADOS" interface, but I have run them under real Ceph locally too, and everything worked as expected.

Related issues

Part of #2142

Future concerns

The RADOS objects handled by this RT are versioned. Should the object layout ever need to change / some backwards-incompatible implementation change, it should be possible to safely do so.

Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

/retest ci/centos/<job-name>: retest the <job-name> after unrelated
failure (please report the failure too!)
/retest all: run this in case the CentOS CI failed to start/report any test
progress or results

gman0 · 2022-02-21T17:50:00Z

Please see a somewhat concrete example of how this would be used here #2142 (comment).

mergify · 2022-02-22T04:55:57Z

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

Madhu-1

Will take a look at it soon.

Madhu-1 · 2022-02-22T06:14:48Z

Makefile

@@ -46,22 +46,19 @@ endif

 GO_PROJECT=github.com/ceph/ceph-csi

+CEPH_VERSION ?= $(shell . $(CURDIR)/build.env ; echo $${CEPH_VERSION})
+# TODO: ceph_preview tag may be removed with go-ceph 0.16.0
+GO_TAGS_LIST ?= $(CEPH_VERSION) ceph_preview


using preview API by default was not accepted see #2633 (comment) we need a stable API as this will be used in production?

Here's a list of preview go-ceph APIs that this PR relies on:

rados.ReadOp.AssertVersion()

rados.ReadOp.GetOmapValuesByKeys() and its return value rados.ReadOpOmapGetValsByKeysStep struct

rados.ReadOp.Read()

rados.WriteOp.AssertVersion()

rados.WriteOp.Remove()

rados.WriteOp.SetXattr()

These are just bare bindings for librados for functions that are stable -- not really any brand new functionality that would be in a big risk of breaking in following two go-ceph releases (the time needed to graduate an API from preview to stable). Is this not acceptable too?

So how do we do this, can anybody please let me know? Can I continue prototyping other parts of #2142 while this is in review, or are you postponing this PR until ceph_preview build tag is no longer needed?

All of these bindings used in this PR are for RADOS primitives, which are very unlikely to change.

am fine with this one as it was not accepted in earlier PR i mentioned it here. @humblec @nixpanic?

fine with me too. 👍

internal/util/rt/errors/errors.go

internal/util/rt/radoswrapper/fakerados.go

internal/util/rt/v1/add.go

gman0 · 2022-03-09T15:26:27Z

/retest ci/centos/mini-e2e-helm/k8s-1.21

gman0 · 2022-03-09T15:26:37Z

/retest ci/centos/mini-e2e-helm/k8s-1.23

gman0 · 2022-03-10T13:34:59Z

/retest ci/centos/mini-e2e-helm/k8s-1.23

gman0 · 2022-03-11T08:20:56Z

/retest ci/centos/mini-e2e-helm/k8s-1.23

humblec · 2022-03-21T11:49:07Z

lgtm. will wait for others to take final look at this.

Cc @ceph/ceph-csi-maintainers

internal/util/reftracker/errors/errors.go

internal/util/reftracker/radoswrapper/fakerados.go

internal/util/reftracker/radoswrapper/interface.go

internal/util/reftracker/radoswrapper/radoswrapper.go

internal/util/reftracker/reftracker.go

internal/util/reftracker/version/version_test.go

gman0 · 2022-03-23T17:36:14Z

/retest ci/centos/mini-e2e/k8s-1.22

internal/util/reftracker/radoswrapper/interface.go

internal/util/reftracker/reftracker.go

internal/util/reftracker/reftype/reftype.go

humblec · 2022-03-24T10:04:03Z

@gman0 this looks good to me, I got a few minor comments/questions though.

gman0 · 2022-03-24T16:47:03Z

/retest ci/centos/mini-e2e-helm/k8s-1.23

gman0 · 2022-03-24T20:53:44Z

/retest ci/centos/mini-e2e-helm/k8s-1.23

gman0 · 2022-03-25T10:35:48Z

Hmm, is the merge stuck?

Rakshith-R · 2022-03-25T10:51:15Z

Hmm, is the merge stuck?

https://github.com/ceph/ceph-csi/pull/2893/checks?check_run_id=5688271877
It's second in the queue,

ceph_preview tag is needed to make new go-ceph's RADOS read/write ops available Signed-off-by: Robert Vasek <robert.vasek@cern.ch>

RT, reference tracker, is key-based implementation of a reference counter. Unlike an integer-based counter, RT counts references by tracking unique keys. This allows accounting in situations where idempotency must be preserved. It guarantees there will be no duplicit increments or decrements of the counter. Signed-off-by: Robert Vasek <robert.vasek@cern.ch>

gman0 · 2022-03-25T14:00:59Z

/retest ci/centos/mini-e2e-helm/k8s-1.21

gman0 · 2022-03-25T14:01:10Z

/retest ci/centos/mini-e2e-helm/k8s-1.22

gman0 · 2022-03-25T14:01:29Z

/retest ci/centos/mini-e2e/k8s-1.21

gman0 · 2022-03-25T14:01:34Z

/retest ci/centos/mini-e2e/k8s-1.22

gman0 · 2022-03-25T15:34:50Z

/retest ci/centos/mini-e2e-helm/k8s-1.21

gman0 · 2022-03-25T15:35:33Z

/retest ci/centos/mini-e2e-helm/k8s-1.22

gman0 · 2022-03-25T15:35:40Z

/retest ci/centos/mini-e2e/k8s-1.21

gman0 · 2022-03-25T15:35:44Z

/retest ci/centos/mini-e2e/k8s-1.22

Rakshith-R · 2022-03-25T15:48:58Z

@Mergifyio refresh
@Mergifyio rebase

mergify · 2022-03-25T15:49:07Z

refresh

✅ Pull request refreshed

ceph-csi-bot · 2022-03-25T15:52:24Z

/retest ci/centos/mini-e2e/k8s-1.21

ceph-csi-bot · 2022-03-25T15:52:25Z

@gman0 "ci/centos/mini-e2e/k8s-1.21" test failed. Logs are available at location for debugging

ceph-csi-bot · 2022-03-25T15:52:25Z

@Mergifyio refresh

mergify · 2022-03-25T15:52:31Z

refresh

✅ Pull request refreshed

gman0 · 2022-03-27T19:23:45Z

@Mergifyio refresh

mergify · 2022-03-27T19:24:18Z

refresh

✅ Pull request refreshed

gman0 mentioned this pull request Feb 21, 2022

cephfs: efficient read-only snapshots as volumes #2142

Closed

gman0 requested a review from a team February 21, 2022 19:36

Madhu-1 reviewed Feb 22, 2022

View reviewed changes

Madhu-1 requested review from humblec and nixpanic February 22, 2022 06:37

gman0 force-pushed the reftracker branch from 5fc22ff to ed32611 Compare February 22, 2022 16:01

gman0 force-pushed the reftracker branch from ed32611 to 4f7800a Compare March 9, 2022 12:31

Madhu-1 reviewed Mar 22, 2022

View reviewed changes

gman0 force-pushed the reftracker branch from 4f7800a to 5d63f20 Compare March 22, 2022 17:43

gman0 requested a review from Madhu-1 March 24, 2022 09:26

humblec reviewed Mar 24, 2022

View reviewed changes

internal/util/reftracker/radoswrapper/interface.go Show resolved Hide resolved

humblec reviewed Mar 24, 2022

View reviewed changes

internal/util/reftracker/reftracker.go Show resolved Hide resolved

humblec reviewed Mar 24, 2022

View reviewed changes

internal/util/reftracker/reftracker.go Outdated Show resolved Hide resolved

humblec reviewed Mar 24, 2022

View reviewed changes

internal/util/reftracker/reftype/reftype.go Outdated Show resolved Hide resolved

gman0 force-pushed the reftracker branch from 5d63f20 to b96820d Compare March 24, 2022 13:53

Madhu-1 approved these changes Mar 25, 2022

View reviewed changes

Madhu-1 requested a review from humblec March 25, 2022 05:15

humblec approved these changes Mar 25, 2022

View reviewed changes

gman0 added 2 commits March 25, 2022 12:04

build: more flexible handling of go build tags; added ceph_preview

416c399

ceph_preview tag is needed to make new go-ceph's RADOS read/write ops available Signed-off-by: Robert Vasek <robert.vasek@cern.ch>

ceph-csi-bot force-pushed the reftracker branch from b96820d to 3fbbdc5 Compare March 25, 2022 12:04

Rakshith-R added the ci/retry/e2e Label to retry e2e retesting on approved PR's label Mar 25, 2022

mergify bot merged commit f6ae612 into ceph:devel Mar 27, 2022

add atomic reference tracker #2893

add atomic reference tracker #2893

Conversation

gman0 commented Feb 21, 2022

Describe what this PR does

Related issues

Future concerns

gman0 commented Feb 21, 2022

mergify bot commented Feb 22, 2022

Madhu-1 left a comment

Choose a reason for hiding this comment

Madhu-1 Feb 22, 2022

Choose a reason for hiding this comment

gman0 Feb 22, 2022

Choose a reason for hiding this comment

gman0 Mar 1, 2022

Choose a reason for hiding this comment

Madhu-1 Mar 2, 2022

Choose a reason for hiding this comment

humblec Mar 21, 2022

Choose a reason for hiding this comment

gman0 commented Mar 9, 2022

gman0 commented Mar 9, 2022

gman0 commented Mar 10, 2022

gman0 commented Mar 11, 2022

humblec commented Mar 21, 2022

gman0 commented Mar 23, 2022

humblec commented Mar 24, 2022

gman0 commented Mar 24, 2022

gman0 commented Mar 24, 2022

gman0 commented Mar 25, 2022

Rakshith-R commented Mar 25, 2022

gman0 commented Mar 25, 2022

gman0 commented Mar 25, 2022

gman0 commented Mar 25, 2022

gman0 commented Mar 25, 2022

gman0 commented Mar 25, 2022

gman0 commented Mar 25, 2022

gman0 commented Mar 25, 2022

gman0 commented Mar 25, 2022

Rakshith-R commented Mar 25, 2022

mergify bot commented Mar 25, 2022

✅ Pull request refreshed

ceph-csi-bot commented Mar 25, 2022

ceph-csi-bot commented Mar 25, 2022

ceph-csi-bot commented Mar 25, 2022

mergify bot commented Mar 25, 2022

✅ Pull request refreshed

gman0 commented Mar 27, 2022

mergify bot commented Mar 27, 2022

✅ Pull request refreshed