-
Notifications
You must be signed in to change notification settings - Fork 91
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
fix: schema not cached in spark/flink loader #334
Conversation
…schema to SchemaCache fix apache#333
Fix bug: SchemaCache init and after query schema then put the latest schema to SchemaCache |
@@ -46,6 +46,7 @@ public SchemaCache(HugeClient client) { | |||
this.propertyKeys = new HashMap<>(); | |||
this.vertexLabels = new HashMap<>(); | |||
this.edgeLabels = new HashMap<>(); | |||
updateAll(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need clear the cache when we firstly create it? (for example?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The HugeGraphSparkLoader does not get the real metadata information when SchemaCache is first created
2 little questions:
|
Codecov Report
@@ Coverage Diff @@
## master #334 +/- ##
============================================
- Coverage 67.49% 67.45% -0.05%
- Complexity 877 878 +1
============================================
Files 86 86
Lines 4024 4028 +4
Branches 475 477 +2
============================================
+ Hits 2716 2717 +1
- Misses 1104 1106 +2
- Partials 204 205 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
1、Hugegraphloader中load()方法会执行this.context.updateSchemaCache()更新元数据 |
hi @haohao0103 ,thanks for your contribution. private LoadContext initPartition(
LoadOptions loadOptions, InputStruct struct) {
LoadContext context = new LoadContext(loadOptions);
for (VertexMapping vertexMapping : struct.vertices()) {
this.builders.put(
new VertexBuilder(context, struct, vertexMapping),
new ArrayList<>());
}
for (EdgeMapping edgeMapping : struct.edges()) {
this.builders.put(new EdgeBuilder(context, struct, edgeMapping),
new ArrayList<>());
}
context.updateSchemaCache();
return context;
} |
我觉得放在这里是可以的,我们第一次就在这里解决的;但是initPartition方法是HugeGraphSparkLoader类私有的;感觉需要调用initPartition方法才能正确正确创建及初始化LoadContext对象,感觉有点不合适哈。个人的浅见。。。。 |
没有明白你的意思,你是指哪里不合适?LoadContext 应该是只有在初始化分区的时候才需要创建? |
我的意思是这样会不会限定了LoadContext对象只有在HugeGraphSparkLoader类才能被正确初始化,限制了扩展性;我们在开发Bulkload代码的时候,需要扩展新的类然后使用LoadContext对象,就没办法调用initPartition方法了;在另外的类中构建LoadContext对象时需要再显示执行下updateSchemaCache()方法才能保证loadcontext对象可以正确使用。这个是我的理解哈,代码设计等我也不是很精通,仅供大佬参考哈 |
@haohao0103 明白你的意思了,但是 |
明白了,谢谢哈 |
我需要重新提交一个pr? |
@haohao0103 可以直接在这个PR上提交代码覆盖 |
…schema to SchemaCache fix apache#333
已提交代码 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
记得也顺便检查一下 flink-loader
那边是否也需要加一下
如果需要 public 的话可以改
需要修改,需要在initBuilders()方法中调用context.updateSchemaCache(); |
private Map<ElementBuilder, List> initBuilders() { |
还可以继续提代码吗? |
fix #333 SchemaCache init and after query schema then put the latest schema to SchemaCache
I have read the CLA Document and I hereby sign the CLA