Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ManagedObjectBackends for HSM #149

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

JBenda
Copy link
Contributor

@JBenda JBenda commented May 9, 2022

Add JManagedBackends a Object Backeand which is at runtime composeble of multiple Object Backends.
JManagedBacends uses a policy module which can be used for HSM on that different object backends.

@JBenda JBenda force-pushed the feature/ManagedObjectBackends branch from bcb85fd to 308b5bb Compare June 9, 2022 11:52
**/
gboolean (*process_access)(gpointer policy_data, const gchar* namespace, const gchar* path, guint obj_id, guint tier, JObjectBackendAccessTypes access, gconstpointer data);

/// short processing period to match new object to a storage tier
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix documentation style.


/**
* Fetches backend where a given object is stored. Also blocks migration for that object.
* Use j_backend_managed_object_open() to allow migration again. If the object not already exists
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean j_backend_managed_close?

gboolean j_backend_managed_object_migrate(JManagedBackends* this, const gchar* namespace, const gchar* path, guint dest);

/**
* If resources a free migrates the specified object to the destination tier, if not return.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are free


/**
* Stops all migrations at all backends, used for maintenance.
* use j_backend_stack_unlock() to re-enable migration
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

j_backend_managed_unlock

* Stops all migrations at all backends, used for maintenance.
* use j_backend_stack_unlock() to re-enable migration
*
* \attention changing object locations or editing them might crashes the policy!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might crash

#include <sys/select.h>
#include <sys/time.h>

#define EXE(cmd, ...) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to have error checking macros on a global level instead of per module. It's probably better to move this one?

gint write_access; ///< true if a write operation is scheduled or running
} RWSpinLock;

/// acquired a read lock
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check doxygen comment style. (Also for following functions in this file.)

}

/// proxy function
gchar const* const* j_configuration_get_object_tiers(JConfiguration* config);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style

"failed to init kv for backend manager!");
this->kv_semantics = j_semantics_new(J_SEMANTICS_TEMPLATE_DEFAULT);

// this->log.filename = j_configuration_get_object_policy_log_file(config);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this line commented?

g_array_unref(this->rw_spin_locks);
g_module_close(this->module);
free(this);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a KV server module loaded in init. Should this one be freed as well?

return TRUE;
}

/// \todo doku
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add documentation (also below).

/**
* Fetches backend where a given object is stored. Also blocks migration for that object.
* Use j_backend_managed_object_open() to allow migration again. If the object not already exists
* use j_backend_managed_object_create() to create it
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open does also create if non-existent as implemented in *.c. Maybe this should be mentioned here. Otherwise someone could think open would fail on non-existant objects (and in worst-case use it as existence check).

EXE(kv_get(this, namespace, path, &entry),
"failed to fetch kventry");
return entry->backend_id;
end:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other parts of JULEA use _error as name for error handling label. Please change to prepare some unified error handling in JULEA.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also in cases below.

@t-erxleben
Copy link
Contributor

Looks good to me, overall.

Some remarks:

  • Why do you use a different logging mechanism as other parts of JULEA?
  • I really like the concept of error checking macros. Maybe we should move them to a global level?
  • Please complete the documentation and check again for typos/grammar in existing documentation.

@t-erxleben
Copy link
Contributor

One general question:
How does this module relate to object distribution (for 'normal' objects) and JDistributedObject? As far as I have understood each JULEA server providing object storage will load this module to handle local tiering. Can it also handle different tiers on different JULEA servers?

@JBenda
Copy link
Contributor Author

JBenda commented Jan 18, 2023

This module makes different ObjectBackends on one server appear to the outside as one ObjectServer, for distributed objects, they will be stripped between the servers as before, and are not effected by this module.

@JBenda JBenda force-pushed the feature/ManagedObjectBackends branch from 46934e2 to ae005e6 Compare January 19, 2023 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants