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

使 ClusterInfo 包含每個節點的 data folder 資訊。 #1106

Closed
garyparrot opened this issue Nov 11, 2022 · 7 comments · Fixed by #1147
Closed

使 ClusterInfo 包含每個節點的 data folder 資訊。 #1106

garyparrot opened this issue Nov 11, 2022 · 7 comments · Fixed by #1147
Assignees

Comments

@garyparrot
Copy link
Collaborator

Context: #1103 (comment)

或許我們把這個方法放到ClusterInfo裡面會更適合,ClusterInfo#nodes可以隨時攜帶這個訊息

@garyparrot
Copy link
Collaborator Author

garyparrot commented Nov 19, 2022

#1108 開始之前會先完成這個 Issue,#1108 有需要自己偽造一個假的叢集,對此這些 Data Folder 的資訊先包含進去 ClusterInfo 會比較順利一點。

或許我們把這個方法放到ClusterInfo裡面會更適合,ClusterInfo#nodes可以隨時攜帶這個訊息

目前 ClusterInfo#nodes 是回傳一個集合的 NodeInfo,我覺得讓 NodeInfo 攜帶這些 folder 資訊不太好,這樣把 equalshashcode 的實作也考慮的話,(host, port, id, folders) 會把 (192.168.0.1, 9092, 3333. [ ... ])(192.168.0.1, 9092, 3333. []) 當成完全不一樣的物件,我是怕如果有人把二種 NodeInfo 混在一起用的話會出現一些很詭異的 bug。

我目前是想做成 ClusterInfo 裡面多一個 method 取代 ClusterInfo#nodes(),原先的 ClusterInfo#nodes() 變成 default method。

default Set<NodeInfo> nodes() {
  return brokerFolders().keySet();
}

Map<NodeInfo, Set<String>> brokerFolders();

基本上 ClusterInfo.of(org.apache.kafka.common.Cluster) 回傳的物件,會 ClusterInfo#brokerFolders() entries 全是 NodeInfo key 但是 value 是全空 Set。然後 Admin#clusterInfo() 回傳的 ClusterInfo#brokerFolders() 會有提供每個 NodeInfo 對應的 data folders

@chia7712 能幫我看看這個概念嗎,謝謝

@chia7712
Copy link
Contributor

目前 ClusterInfo#nodes 是回傳一個集合的 NodeInfo,我覺得讓 NodeInfo 攜帶這些 folder 資訊不太好,這樣把 equals 和 hashcode 的實作也考慮的話,(host, port, id, folders) 會把 (192.168.0.1, 9092, 3333. [ ... ]) 和 (192.168.0.1, 9092, 3333. []) 當成完全不一樣的物件,我是怕如果有人把二種 NodeInfo 混在一起用的話會出現一些很詭異的 bug。

同意

基本上 ClusterInfo.of(org.apache.kafka.common.Cluster) 回傳的物件,會 ClusterInfo#brokerFolders() entries 全是 NodeInfo key 但是 value 是全空 Set。然後 Admin#clusterInfo() 回傳的 ClusterInfo#brokerFolders() 會有提供每個 NodeInfo 對應的 data folders

這個概念不錯,或者更簡化一點,我們讓NodeInfo多一個方法可以回傳Set<String>,然後預設是回傳空陣列(如果是從org.apache.kafka.common.Cluster來的),這樣大部分的地方都不用更改,只要在建構NodeInfo的時候可以多吃一個參數就好

@garyparrot
Copy link
Collaborator Author

這個概念不錯,或者更簡化一點,我們讓NodeInfo多一個方法可以回傳Set,然後預設是回傳空陣列(如果是從org.apache.kafka.common.Cluster來的),這樣大部分的地方都不用更改,只要在建構NodeInfo的時候可以多吃一個參數就好

所以 NodeInfoequalshashcode 不需要去包含這個集合嗎,我是擔心在某個地方的聚合操作會讓其中一個物件帶有的資訊消失。比如下面這段程式碼

public class Ignore {

  public static class NodeInfo {
    public final String host;
    public final int port;
    public final int id;
    public final Set<String> folders;

    public NodeInfo(String host, int port, int id, Set<String> folders) {
      this.host = host;
      this.port = port;
      this.id = id;
      this.folders = folders;
    }

    @Override
    public boolean equals(Object o) {
      if (this == o) return true;
      if (o == null || getClass() != o.getClass()) return false;
      NodeInfo nodeInfo = (NodeInfo) o;
      return port == nodeInfo.port && id == nodeInfo.id && host.equals(nodeInfo.host);
    }

    @Override
    public int hashCode() {
      return Objects.hash(host, port, id);
    }

    @Override
    public String toString() {
      return "NodeInfo{" +
          "host='" + host + '\'' +
          ", port=" + port +
          ", id=" + id +
          ", folders=" + folders +
          '}';
    }
  }

  public static void main(String[] args) {
    NodeInfo broker3333_1 = new NodeInfo("host", 9092, 3333, Set.of());
    NodeInfo broker3333_2 = new NodeInfo("host", 9092, 3333, Set.of("/ssd1", "/ssd2"));

    Set<NodeInfo> set12 = Stream.of(broker3333_1, broker3333_2).collect(Collectors.toUnmodifiableSet());
    Set<NodeInfo> set21 = Stream.of(broker3333_2, broker3333_1).collect(Collectors.toUnmodifiableSet());
    System.out.println(set12);
    System.out.println(set21);
    System.out.println();
    if(!set12.toString().equals(set21.toString()))
      throw new IllegalStateException("Information loss");

  }

}
[NodeInfo{host='host', port=9092, id=3333, folders=[]}]
[NodeInfo{host='host', port=9092, id=3333, folders=[/ssd1, /ssd2]}]

Exception in thread "main" java.lang.IllegalStateException: Information loss
	at org.astraea.common.balancer.Ignore.main(Ignore.java:74)

因為 NodeInfoStream 裡面的順序不同,在 set12 的順序中,data folder 的資訊不見了。

@chia7712
Copy link
Contributor

chia7712 commented Nov 19, 2022

所以 NodeInfo 的 equals 和 hashcode 不需要去包含這個集合嗎,我是擔心在某個地方的聚合操作會讓其中一個物件帶有的資訊消失。比如下面這段程式碼

蠻有道理的,由於NodeInfo作為 key 是很常見的事情,這的確是個風險

@chia7712
Copy link
Contributor

Map<NodeInfo, Set> brokerFolders();

現在要討論的是怎麼填寫這個方法,看起來有兩種:

  1. 建構ClusterInfo的時候直接填寫,沒寫的話就回傳空陣列
  2. 嘗試將NodeInfo 轉型成Broker,如果成功則從中取得Path並整理成brokerFolders

第二個方法的好處是如果我們之後要添加除了brokerFolders以外的東西時,可以不用變更ClusterInfo的建構方式,只需要在Broker身上添加,然後在撰寫 helper 就好

@garyparrot
Copy link
Collaborator Author

我覺得方法 1 的特色是他和真實的節點比較抽離,但大概要和真實叢集之間多一段轉換的 code,方法 2 讓 ClusterInfo 和真實的叢集物件 Broker 黏的比較緊密。

用方法 2 可能改動可以少一點,黏太緊我不確定會不會有問題,不過等問題顯現的時候再轉去用方法 1 應該也沒問題。

或許我先用方法 2 做,如果遇到問題再討論。

@chia7712
Copy link
Contributor

@garyparrot 這兩個方法我覺得都可以,由你決定要先採取哪個作法,如果有碰到技術問題我們再討論

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 a pull request may close this issue.

2 participants