ctubbsii edited a comment on issue #1837:
URL: https://github.com/apache/accumulo/issues/1837#issuecomment-744663523
> in such a way that T1C.T1 and SC.T1 are ignored in favor of T2C.T1
@hlgp I'm a little confused by your nomenclature, so let me translate first
to make sure I understand, then I'll address it:
Imagine a config key: `table.custom.table1.key`
* In the system configuration, the value is set to `v0`
* In `table1`'s configuration, the value is set to `v1`
* In `table2`'s configuration, the value is set to `v2`
* In `table3`'s configuration, no value is set
It sounds like what you're saying is that if `table2` sets
`table.custom.table1.key = v2`, then operations performing in the context of
`table1` will see the `v2` value instead of `v0` or `v1`. Is that what you are
saying?
If so, that is definitely not the case. Operations performing in the context
of `table1` will see `v1`, operations performing in the context of `table2`
will see `v2`, and operations performing in the context of `table3` will see
`v0`.
The fact that the key contains the name of `table1` has no bearing on
whether its value will be seen in any particular context. In general, the only
thing that matters is where the configuration is set.
The bug with the `HostRegexTableLoadBalancer` was a special case. It is
*not* the case that the configuration of one table was affecting the context of
another table. Rather, it was because that balancer was explicitly switching to
each table's context and incorporating any config it found anywhere. The config
wasn't affecting other contexts... the balancer was reading from all contexts.
The current behavior fixes that bug, and only reads from the system context.
Additionally, it logs warnings if it finds these properties in any per-table
context, so that if something was working before and isn't after an upgrade,
users will have some clue as to why.
This balancer should never have used the `table.custom.` prefix, as these
properties were never intended to be stored in per-table configuration. They
were intended to be stored in system configuration only... like other
properties with the `general.custom.` prefix. I suspect that these predated the
addition of `general.custom.` properties, and that's why it worked the way it
did. This bug persists in 1.10, but the use of this balancer is a pretty niche
use case, and there is a workaround. We could implement a simple fix in 1.10,
like @ivakegg suggested, but nobody has done that. It would be a slight
behavior change that could unexpectedly break people (while fixing others), so
I'm not sure if it's worth it in a bugfix release.
So, I hope I understood what you were saying, and that my explanation makes
sense. My main point is that I think this was an issue of
`HostRegexTableLoadBalancer`, specifically, and not a general problem.
Furthermore, it has been fixed in 2.1 development, and there is a workaround in
1.10. If somebody submits a PR to fix it for 1.10, feel free to ping me on it
and I'd be happy to review it. I think it might just involve adding a line to
`if (!tableName.equals(table.getKey())) { continue; }` after
https://github.com/apache/accumulo/blob/56142a89952533fef922fa86739a879c073e7c2a/server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java#L244
However, I have not experimented or tested with this. It's just a guess from
glancing at the code, and would probably need some sort of unit test case to go
with it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
