Simplify pmemstream API

[karczex]

FEAT: Simplify pmemstream API

Rationale

The fact the particular entry is located in the particular region which is part of particular stream instance should be reflected in API.

Currently functions in pmemstream unnecessary gets to many arguments. i.e: int pmemstream_append(struct pmemstream *stream, struct pmemstream_region region, struct pmemstream_region_runtime *region_runtime, const void *data, size_t size, struct pmemstream_entry *new_entry) which is error prone. It's super easy to pass mismatched entry, region and region runtime.

Description

• Make region and entry structures purely runtime.
• Use pointers to regions and entries instead of offsets (allow to remove a lot of offset_to_ptr calls).
• Add serialization/deserialization functions, which would operate on offsets.

API Changes

Signature change

int pmemstream_region_allocate(struct pmemstream *stream, size_t size, struct pmemstream_region *region);

int pmemstream_region_free(struct pmemstream_region region);
size_t pmemstream_region_size( struct pmemstream_region region);


size_t pmemstream_region_usable_size(struct pmemstream_region region);


int pmemstream_reserve(struct pmemstream_region region, size_t size,
		       struct pmemstream_entry *reserved_entry);


int pmemstream_publish(struct pmemstream_entry reserved_entry);

int pmemstream_append(const struct pmemstream_region region, const void *data, size_t size,
		      struct pmemstream_entry *new_entry);

int pmemstream_async_publish(struct pmemstream_entry entry);

int pmemstream_async_append(struct vdm *vdm, const struct pmemstream_region region, const void *data, size_t size,
			    struct pmemstream_entry *new_entry);

uint64_t pmemstream_committed_timestamp(struct pmemstream *stream);

uint64_t pmemstream_persisted_timestamp(struct pmemstream *stream);

struct pmemstream_async_wait_fut pmemstream_async_wait_committed(struct pmemstream *stream, uint64_t timestamp);

struct pmemstream_async_wait_fut pmemstream_async_wait_persisted(struct pmemstream *stream, uint64_t timestamp);

const void *pmemstream_entry_data(struct pmemstream_entry entry);

size_t pmemstream_entry_size(struct pmemstream_entry entry);

uint64_t pmemstream_entry_timestamp(struct pmemstream_entry entry);

Functions to be removed

	pmemstream_region_runtime_initialize()

Functions to be added

Those functions are needed to store as a separate metadata not related to actual address

	size_t pmemstream_region_offset(const struct pmemstream_region);
	size_t pmemstream_entry_offset(const struct pmemsream_entry);

	int pmemstream_reigon_from_offset(size_t offset, struct pmemstream_region *region); // Check if it's region is needed, so need to read metadata from pmem

	int pmemstream_entry_from_offset(size_t offset, struct *pmemstream_entry); // Check if it's entry is needed, so need to read metadata from pmem

Implementation details

Change structures pmemstream_region and pmemstream_entry to be purely runtime and operate on pointers instead of offsets

/* Runtime structure */
struct pmemstream_region {
	struct pmemstream *stream;

	struct pmemstream_region_runtime *region_runtime;

	/* Pointer to region on pmem */
	void *data;
};

/* Runtime structure */
struct pmemstream_entry {

	struct pmemstream_region region;

	 /* raw pointer to entry on pmem */
	 void *data;

	/* possible optimization: store in runtime structure size and timestamp to minimize entry metadata
	 * reads from pmem */
	size_t size;
	uint64_t timestamp;
};

[igchor]

1. Perhaps functions that take region as the first parameter should have pmemstream_region prefix? E.g. pmemstream_region_append instead of pmemstream_append?

2. Instead of extending/changing reserve/publish function we might think about creating some more general transactional API, so that publish can actually publish multiple entries at once. Something like this perhaps:

struct pmemstream_tx tx = pmemstream_tx_begin(stream);
pmemstream_tx_region_reserve(tx, region1, size1, &entry1);
pmemstream_tx_region_reserve(tx, region1, size2, &entry2);
pmemstream_tx_region_reserve(tx, region2, size3, &entry3);
// fill the entries ...
pmemstream_tx_commit(tx);

But I'm not sure if this is the best idea. Currently, publish stores all the metadata, so if we could like to do that on tx_commit we would need to keep some list of entries inside tx.

3. There should be no dynamic allocations for pmemstream_region nor pmemstream_entry. pmemstream_entry should hold region directly, not a pointer to it.

[lukaszstolarczuk]

1. Perhaps functions that take region as the first parameter should have pmemstream_region prefix? E.g. pmemstream_region_append instead of pmemstream_append?

I think it may be misleading - one would think that you want to "append a new region to stream" 😉 I think it's ok, like @karczex proposed.

[karczex]

3. There should be no dynamic allocations for pmemstream_region nor pmemstream_entry. pmemstream_entry should hold region directly, not a pointer to it.

That's really good point.