aws.S3_PING extends the FILE_PING and thus exposes it's configuration that is never used which might be confusing

[rhusar]

e.g. location is exposed as /tmp/jgroups which can be confusing to users.

[standalone@localhost:9990 /] /subsystem=jgroups/channel=ee/protocol=aws.S3_PING:read-resource(include-runtime=true
{
    "outcome" => "success",
    "result" => {
        "acl_grant_bucket_owner_full_control" => false,
        "after_creation_hook" => undefined,
        "async_discovery" => false,
        "async_discovery_use_separate_thread_per_request" => false,
        "break_on_coord_rsp" => true,
        "bucket_name" => "rhusar",
        "bucket_prefix" => "",
        "check_if_bucket_exists" => true,
        "current_coord" => "localhost",
        "discovery_rsp_expiry_time" => 60000L,
        "endpoint" => undefined,
        "ergonomics" => true,
        "id" => 790,
        "info_writer_max_writes_after_view" => 2,
        "info_writer_running" => false,
        "info_writer_sleep_time" => 10000L,
        "is_coord" => true,
        "level" => undefined,
        "local_addr" => "localhost",
        "location" => "/tmp/jgroups",
        "max_members_in_discovery_request" => 500,
        "max_rank_to_reply" => 0,
        "num_discovery_requests" => 1,
        "num_discovery_runs" => 1,
        "path_style_access_enabled" => false,
        "policies" => "n/a",
        "reads" => 1,
        "region_name" => "eu-central-1",
        "register_shutdown_hook" => true,
        "remove_all_data_on_view_change" => false,
        "remove_old_coords_on_view_change" => false,
        "return_entire_cache" => false,
        "send_cache_on_join" => true,
        "sends_can_block" => false,
        "stagger_timeout" => 0L,
        "stats" => false,
        "transport_supports_multicasting" => true,
        "update_store_on_view_change" => true,
        "use_disk_cache" => false,
        "view" => "[localhost|7]",
        "write_data_on_find" => false,
        "writes" => 2
    }
}

[belaban]

What do you suggest?

How does the CLI fetch the attributes? Does it get all fields/getters with annotations ManagedAttribute / Property?

[rhusar]

What do you suggest?

@belaban So far I have only come up with unsetting the irelevant attributes to null / don't use values (as those are primitive types so cannot be null). Any better ideas? :)

diff --git a/src/main/java/org/jgroups/protocols/aws/S3_PING.java b/src/main/java/org/jgroups/protocols/aws/S3_PING.java
index da64a77..b15d30e 100644
--- a/src/main/java/org/jgroups/protocols/aws/S3_PING.java
+++ b/src/main/java/org/jgroups/protocols/aws/S3_PING.java
@@ -109,6 +109,12 @@ public class S3_PING extends FILE_PING {
     public void init() throws Exception {
         super.init();

+        // These @Property-ies are inherited from FILE_PING but are never used in the S3_PING implementation.
+        // Set these to values that may hint the user that these are not in use.
+        location = null;
+        num_read_loops = -1;
+        read_sleep = -1;
+
         if (bucket_prefix == null || bucket_prefix.equals("/")) {
             bucket_prefix = "";
         } else if (!bucket_prefix.endsWith("/") && !bucket_prefix.isEmpty()) {

How does the CLI fetch the attributes? Does it get all fields/getters with annotations ManagedAttribute / Property?

Yes, this is all automatically generated, there is no special handling/manual intervention per protocol (that would be too cumbersome to maintain).

[belaban]

We could introduce an annotation (on the class) which - similar to Property.exposeAsManagedAttribute - hides some attributes/properties. But again, this depends on how you fetch them...

[pferraro]

@belaban Currently, we register all protocol fields annotated with @ManagedAttribute or @Property(exposeAsManagedAttribute=true)

[rhusar]

@belaban Currently, we register all protocol fields annotated with @ManagedAttribute or @Property(exposeAsManagedAttribute=true)

For reference
https://github.com/wildfly/wildfly/blob/main/clustering/jgroups/extension/src/main/java/org/jboss/as/clustering/jgroups/subsystem/ProtocolMetricsHandler.java#L261-L278

[belaban]

Hmm... add something like this below?

@DontExpose("location","num_read_loops","read_sleep")
class S3_PING {
   ...
}

Not really nice, but other than changing the inheritance I can't think of any other solutions.

[rhusar]

@belaban I am afraid that's not worth it and should really be fixed by fixing inheritance - not extend FILE_PING but FILE_PING should be split to abstract class with the shared marshalling read/write concepts and protocols like FILE_PING, S3_PING and AZURE_PING should extent that base abstract class. If I open such PR would that be mergeable from your side Bela?

[belaban]

Sounds like a plan